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

[InferRW] Remove dependence of write-mode on enable for memory #6818

Merged
merged 2 commits into from Mar 14, 2024

Conversation

prithayan
Copy link
Contributor

The read-write port of a memory has an input enable en signal and a write-mode wmode signal. The en signal enables the port for either read/write mode. The wmode is enabled to set the memory in the write mode and disabled to read from the memory.
This PR removes the dependency of wmode on the en signal of a read-write port of a memory.
The wmode signal matters only if the en is enabled, hence traverse the expression tree for the wmode and replace en with a constant 1.

@prithayan prithayan marked this pull request as draft March 13, 2024 15:21
@prithayan prithayan changed the title [InferRW] Remove dependence of WMODE on EN [InferRW] Remove dependence of write-mode on enable for memory Mar 13, 2024
@prithayan prithayan marked this pull request as ready for review March 13, 2024 16:25
@prithayan
Copy link
Contributor Author

@seldridge , @debs-sifive , I checked that this PR fixes the issue with the redundant AND for write-mode with the enable. Its ready for review.

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.

I don't know much about this pass, or if there is a better way to solve the problem at a high level. The implementation seems fine to me.

@prithayan prithayan merged commit 315892f into main Mar 14, 2024
4 checks passed
@prithayan
Copy link
Contributor Author

Merged this now for the next release, post merge comments are welcome.

prithayan added a commit that referenced this pull request Mar 16, 2024
This fixes a bug in InferReadWrite, the builder was not respecting dominance when updating the IR.
Ensure that the builder creates the constant at the start of the module.
The bug was introduced in #6818
@prithayan prithayan deleted the dev/pbarua/simplify_mem branch March 21, 2024 23:12
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.

None yet

2 participants