Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Remove multi-dimensional memories from core primitives #907

Open
rachitnigam opened this issue Feb 10, 2022 · 8 comments
Open
Labels
C: Library Calyx's standard library S: Available Can be worked upon

Comments

@rachitnigam
Copy link
Contributor

I've pondering this for a while. If I understand correctly, multi-dimensional memories are not directly mappable when doing synthesis, which probably means that at some point, the underlying synthesis toolchain remaps that accesses to multi-dimensional memories to single dimension ones.

If this is true, I think we should move multi-dimensional memories out of the core primitives and implement a pass that lower multi-dimensional memories into single dimensional memories to make the cost of this transformation explicit.

I'd also like the CIRCT folks to chime in if possible: @mikeurbach @stephenneuendorffer

@rachitnigam rachitnigam added S: Discussion needed Issues blocked on discussion C: Library Calyx's standard library labels Feb 10, 2022
@stephenneuendorffer
Copy link

Makes sense to me. This is one of those things that is likely an important hardware lowering for many platforms.

@mikeurbach
Copy link
Collaborator

I agree, it seems best to make this explicit in the IR rather than deferring to an external toolchain. I was thinking of something similar, in the context of scheduling. Lowering from a multi-dimensional memory to a single dimension may introduce hardware components related to linearizing indices, and these should definitely be taken into account in scheduling, or anything else with a cost model.

Perhaps it would be nice to add the transformation first and deprecate multi-dimensional memories, before fully removing them? In CIRCT, @mortbopet has developed a pass that does exactly this, so we already have this option coming in to Calyx from CIRCT workflows.

@EclecticGriffin
Copy link
Collaborator

Also makes sense to me! (fwiw the interpreter & debugger use flattened versions internally anyway)

@sampsyo
Copy link
Contributor

sampsyo commented Feb 11, 2022

Indeed! One baby step we could take here would be reimplementing the std_mem_d* components in pure Calyx, which would let us remove the primitives while preserving compatibility… except that we don't yet have any support for parameterized Calyx components.

@cgyurgyik
Copy link
Collaborator

except that we don't yet have any support for parameterized Calyx components.

Yeah I recall this being brought up awhile back, with this being a main limitation.

@rachitnigam
Copy link
Contributor Author

We don’t have to implement them in Calyx if we can’t. We should move them out of the core primitives.

@cgyurgyik
Copy link
Collaborator

cgyurgyik commented Feb 11, 2022

Isn't this a common enough abstraction though that will take some burden off the frontend generators? Like I think the right approach is templated memories with a correspond pass to flatten them, rather than entirely remove them.

@rachitnigam
Copy link
Contributor Author

rachitnigam commented Feb 11, 2022

No I literally mean put them in a different file that is not core. When we have all the other fancy stuff we can use it but core should be tiny.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Library Calyx's standard library S: Available Can be worked upon
Projects
None yet
Development

No branches or pull requests

6 participants