[PATCH] D87778: [MemorySSA] Be more conservative when traversing MemoryPhis.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/include/llvm/Analysis/MemorySSA.h:1262
         if (Translator.getAddr() != Location.Ptr) {
-          CurrentPair.second = Location.getWithNewPtr(Translator.getAddr());
+          CurrentPair.second = Location.getWithNewPtr(Translator.getAddr())
+                                   .getWithNewSize(LocationSize::unknown());
----------------
asbirlea wrote:
> I'm not clear that the size should be always unknown after phi translation with different ptr, but feel free to correct me.
> Here's my suggestion.
> ```
> if (Translator.getAddr() != Location.Ptr) {
>   CurrentPair.second = CurrentPair.second.getWithNewPtr(Translator.getAddr());
>   if (PerformedPhiTranslation)
>             *PerformedPhiTranslation = true;
> } else {
>   CurrentPair.second = Location.getWithNewSize(LocationSize::unknown());
> }
> ```
Hm, I think if we translate to a different address, we still should set to an unknown size, as the translated address could itself be loop dependent and reference multiple actual locations.

But if we translate to an invariant address, it should probably be safe to use the precise address?

The invariance criteria in the patch is just a few things that matter in practice and are easy to check, but I am sure there is potential for detecting more cases there, after we established a safe baseline


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87778/new/

https://reviews.llvm.org/D87778