search

calyxir / calyx

Intermediate Language (IL) for Hardware Accelerator Generators
https://calyxir.org
MIT License
450 stars 44 forks source link

place compile-invoke after static-promotion #2032

Closed paili0628 closed 2 weeks ago

paili0628 commented 2 weeks ago

place compile-invoke after static-promotion.

This would possibly lead to a few performance drops. Hence this pr aims to investigate into the reasons for the performance drops and try to fix them.

calebmkim commented 2 weeks ago

Looking at the test cases, they're failing because static-promotion doesn't want to promote invoke with comb group. But I think that was because those comb groups used to increase the latency (bc they would write into a register). Now I don't think that restriction should apply. In other words, I think it's fine to get rid of that check. Does that make sense?

paili0628 commented 2 weeks ago

Looking at the test cases, they're failing because static-promotion doesn't want to promote invoke with comb group. But I think that was because those comb groups used to increase the latency (bc they would write into a register). Now I don't think that restriction should apply. In other words, I think it's fine to get rid of that check. Does that make sense?

Wait I don't think we currently have that restriction in static-promotion?

rachitnigam commented 2 weeks ago

invoke-with should not increase latency anymore; compile-invoke removes the full construct and inlines the comb group assignments.

calebmkim commented 2 weeks ago

@paili0628 in case you haven't figured out the bug, I did some digging and I think it's here:

https://github.com/calyxir/calyx/blob/652b761b5136dce18401629cd8498ff1013d8717/calyx-opt/src/analysis/read_write_set.rs#L243

comp.go should be added to outs. I can add a PR to fix this bug unless you want to do it.