[PATCH] D84905: [MemorySSA] Restrict optimizations after a PhiTranslation.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 3 11:17:36 PDT 2020
fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.
LGTM, thanks for adding he comments.
================
Comment at: llvm/lib/Analysis/MemorySSA.cpp:525
DenseSet<ConstMemoryAccessPair> VisitedPhis;
+ bool PerformedPhiTranslation = false;
----------------
asbirlea wrote:
> fhahn wrote:
> > might be worth adding a comment, describing what it means (and when it will be reset).
> >
> > Also, would it make sense to move this to a datastructure already tied to a set of paths that are explored together, e.g. the `UpwardsMemoryQuery`? The current approach seems fine to me, I am just wondering if we could reduce the scope of the variable a bit (and avoid relying on the reset calls).
> I believe the current approach does narrow the scope. It's reseting the state after optimizing towards a phi, which is the join point where we need to stop optimizing in the presence of a phi translation. The `UpwardsMemoryQuery` contains info about the query itself, and it is set at the start of the search (in `findClobber`). The scope of the bool needs to be similar to that of VisitedPhis, that of internal bookkeeping for a search, and very much related to phi optimization.
>
> I'll add a comment to clarify.
Thanks, agreed it seems in line with the way caching/invalidation is done throughout here and changing that would require bigger changes without obvious benefit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84905/new/
https://reviews.llvm.org/D84905
More information about the llvm-commits
mailing list