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

[Calyx] Switch sequential memories to be true single port memories #6765

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

andrewb1999
Copy link
Contributor

Currently, Calyx seq memories have read enable/done and write enable/done signals. This gives the wrong impression that you can perform simultaneous reads and writes, like how you could with a simple dual port memory, but the intention is for seq memories to be single port (as they have a single set of addresses). This PR changes seq mems to have a single content enable/done interface, with a write enable signal that is not a go signal. The intended way to perform a read is the set content enable high and write enable low (and wait for content done to go high). For a write we do the same thing but set the write enable high.

This interface more closely matches how real hardware designers implement single port memories.

These changes need to be merged simultaneously with a new Calyx release with matching changes in the native compiler. See PR with native changes here: calyxir/calyx#1610

Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this in lockstep with the native Calyx compiler makes sense to me. I think the implementation looks great.

@rachitnigam
Copy link
Contributor

Awesome! I think we should merge this since main on Calyx already has the required changes. I think after this release of the Calyx compiler, we should discuss a better approach towards maintaining both of these toolchains together and lock-stepping release cycles.

@mikeurbach
Copy link
Contributor

👍 sounds good!

@andrewb1999 andrewb1999 merged commit 2d58838 into llvm:main Feb 29, 2024
4 checks passed
@andrewb1999 andrewb1999 deleted the true-single-port branch March 15, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants