[InferReadWrite] Add heuristic to infer unmasked memory #6790
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
uenoku
reviewed
Mar 6, 2024
| // If we can infer that all the bits of the mask are always assigned | ||
| // the same value, then the memory is unmasked. | ||
| if (auto maskVal = getConnectSrc(sf)) { | ||
| SmallVector<Value> bits; |
There was a problem hiding this comment.
I'm not sure it's reasonable to use Value to represent bit precise states. I think llvm::KnownBits would be right better fit here. You may want to refer the similar analysis for comb
circt/lib/Dialect/Comb/CombAnalysis.cpp
Line 86 in e1a69e3
There was a problem hiding this comment.
Thanks @uenoku , the analysis is very similar, but seems like its only trying to figure out the 1s and 0s in the bits.
But in this PR, the analysis is different.
- In general the mask value is going to be constructed by concat of few 1 bit values, which may not be constants.
- If the same Value is used to mask each field of an aggregate memory data, then each bit of the mask can be traced back to that Value.
- So, this unmasked inference analysis is only trying to follow the dataflow into each of the bits of an Value to a source Value and then check if the source value is same for each bit.
I agree about the recursion depth, I will make this an iterative algorithm.
prithayan
force-pushed
the
dev/pbarua/infer-unmasked
branch
from
67a3e12
to
3c28356
Compare
prithayan
requested review from
darthscsi,
dtzSiFive and
seldridge
as code owners
|
@seldridge , @debs-sifive This PR updates the heuristic to infer an unmasked memory, ready for a review. |
mikeurbach
reviewed
Mar 14, 2024
prithayan
force-pushed
the
dev/pbarua/infer-unmasked
branch
from
4169217
to
f6cc014
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates the heuristic to infer an unmasked memory.
If all the bits of a masked memory are exactly the same, then it can be replaced with an unmasked memory.
This is an attempt to fix a use case, in which
firtoolintroduces masked memory for an aggregate data type when the user expected an unmasked one.