[PATCH] D35439: Make promoteLoopAccessesToScalars independent of AliasSet [NFC]

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea marked an inline comment as done.
asbirlea added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:309
+        for (const auto &ASI : AS) {
+          PointerMustAliases.insert(ASI.getValue());
+        }
----------------
sanjoy wrote:
> Can you see duplicates here?  If not, I'd suggest changing this to a `SmallVector` (and pass in an `ArrayRef` to `promoteLoopAccessesToScalars`).
I'm not 100% sure if I can see duplicates, but the way I see the AliasSets being built it looks possible.  The values are stored as a list in AliasSet:PointerRec.

The actual reason for keeping the PointerMustAliases as a set, is that in the alternative implementation I have (using MemorySSA)  values can be deleted.
This happens to enable promotion after promotion.
In the AliasSetTracker, when this case comes up, there's the callback in LoopPromoter that updates the AliasSet. With MemSSA, something similar updates a SmallPtrSet<Value*>.

FWIW, I'm not planning to land this until I get the follow-up patch up for review and get input on both.


https://reviews.llvm.org/D35439