dillonhuff / clockwork

A polyhedral compiler for hardware accelerators
56 stars 12 forks source link

Index Variable Need by Compute #113

Closed joyliu37 closed 4 years ago

joyliu37 commented 4 years ago

I am debugging camera pipeline now and get a coreir generation error, when call this loop block. https://github.com/dillonhuff/clockwork/blob/878ed57ee7919960791b6fbc4d8e4bc267fdb901/coreir_backend.cpp#L1246-L1250

It said that it could not find the exe_start_control_vars. I am wondering this control singal is not generated since I disable most of the control logic. Could you show me where is this wire is created? Thanks

dillonhuff commented 4 years ago

@joyliu37 right, that wire comes out of an on-fabric controller that I generate here: https://github.com/dillonhuff/clockwork/blob/11502475146caf70df1e7da8069c298ece88fd20/coreir_backend.cpp#L1800-L1805

joyliu37 commented 4 years ago

So camera pipeline computation stage need the index information as well? So we need to generate the it and feed in? OK. I think I need to remerge master again. This changes made a day ago. Are you refactoring the controller generation into this function?

dillonhuff commented 4 years ago

@joyliu37 yes demosaicing needs the loop index variables so we need to send the index variables from an on-fabric controller to the compute unit.

Yes I am refactoring the codegen because Tony and I are adding support for the other memory tile variants we are going to need for the paper.

joyliu37 commented 4 years ago

Cool, let me pull the changes and add that index controller.

joyliu37 commented 4 years ago

I saw you make some changes in cgralib.cpp, In the current memory tile there are no chain_data_output. I want to make some changes to it. Is there a sanity check test that I need to run beside cgra-flow and travis-tests that make sure I did not break your test? I think you may create some dual port memory tests this weekend.

dillonhuff commented 4 years ago

@joyliu37 I am making some new tests. I just pushed the most recent version with resnet and mobilenet using the dual port, external addrgen tile.

As long as cgra-flow and travis-tests pass your changes are ok.

joyliu37 commented 4 years ago

Oops, it seems trigger a coreIR select error in resnet with chaining. Let me dig into that.

dillonhuff commented 4 years ago

Which resnet is breaking? The resnet in my tests that uses the dual port tile or the resnet in your branch that uses the wide-fetch tile?

joyliu37 commented 4 years ago

Umm, your resnet test. I would add index to the chain_data_in and chain_data_out since they will associate with each port.

joyliu37 commented 4 years ago

OK. I just add a if guard, it seems that you always use dual port SRAM with external controller. I put your chaining definition under there.

dillonhuff commented 4 years ago

@joyliu37 cool. Sounds good.

joyliu37 commented 4 years ago

I am a little curious, did you implement chaining by yourself? I did not see you link any lake stuff.

dillonhuff commented 4 years ago

@joyliu37 yeah for the SRAM + external address gen I just built in on the fabric:

https://github.com/dillonhuff/clockwork/blob/64af42c1606e6f9bd7dee80e802732d524108c6a/coreir_backend.cpp#L4079-L4104

dillonhuff commented 4 years ago

@joyliu37