Cache ReadsWrites instances for alias analysis#5472
Open
atta-ullah01 wants to merge 1 commit intop4lang:mainfrom
Open
Cache ReadsWrites instances for alias analysis#5472atta-ullah01 wants to merge 1 commit intop4lang:mainfrom
atta-ullah01 wants to merge 1 commit intop4lang:mainfrom
Conversation
atta-ullah01
force-pushed
the
fix/cache-readswrites-instances
branch
from
407005d to
255b8fe
Compare
asl
reviewed
Feb 2, 2026
asl
reviewed
Feb 2, 2026
atta-ullah01
force-pushed
the
fix/cache-readswrites-instances
branch
from
255b8fe to
5a84e22
Compare
fruffy
added
the
core
asl
reviewed
Feb 17, 2026
frontends/p4/sideEffects.cpp
Outdated
| GetWrittenExpressions gwe(typeMap); | ||
| gwe.setCalledBy(this); | ||
| mce->apply(gwe, getContext()); | ||
| auto cacheIt = writtenExpressionsCache.find(orig); |
Contributor
There was a problem hiding this comment.
Will you please use lazy_emplace here? This way only a single map lookup will be done and the map entry will be constructed in-place. Something like this:
modifies = writtenExpressionsCache.lazy_emplace(orig,
[this,mce,orig](const auto &ctor) {
GetWrittenExpressions gwe(typeMap);
gwe.setCalledBy(this);
mce->apply(gwe, getContext());
ctor(orig, gwe.written);
})->second;
asl
reviewed
Feb 17, 2026
frontends/p4/sideEffects.cpp
Outdated
| gwe.setCalledBy(this); | ||
| mce->apply(gwe, getContext()); | ||
| writtenExpressionsCache[orig] = gwe.written; | ||
| modifies = gwe.written; |
Contributor
There was a problem hiding this comment.
I don't think you want to copy the whole set here. Maybe you'd just use pointer here (be careful though as this will be pointer to map entry so it could be invalidated on every insertion)
Contributor
Author
There was a problem hiding this comment.
changed modifies to a const auto &, the reference is safe now since lazy_emplace handles both find/insert in one call
atta-ullah01
force-pushed
the
fix/cache-readswrites-instances
branch
from
5a84e22 to
bcc7df3
Compare
asl
force-pushed
the
fix/cache-readswrites-instances
branch
from
bcc7df3 to
0dd29ee
Compare
asl
added
run-validation
Persist ReadsWrites instances in DoSimplifyExpressions and RemoveAliases instead of recreating them on every mayAlias() call. This reuses the internal lookup cache across multiple checks. Also cache GetWrittenExpressions results to avoid recomputing them for the same MethodCallExpression. Clear both caches in end_apply() to allow GC and prevent stale entries. Signed-off-by: Attaullah Ansari <mdattaullahansari152@gmail.com>
asl
force-pushed
the
fix/cache-readswrites-instances
branch
from
0dd29ee to
1a158f0
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Addresses the FIXMEs in sideEffects.cpp and copyStructures.cpp noting that ReadsWrites was being recreated on every mayAlias() call, losing all declaration lookup caching.
Now ReadsWrites is kept as a class member and reused across calls. Also added a cache check in ReadsWrites::get() to return early when we've already analyzed an expression, and cached GetWrittenExpressions results per MethodCallExpression.