[PATCH] D84905: [MemorySSA] Restrict optimizations after a PhiTranslation.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: llvm/lib/Analysis/MemorySSA.cpp:612
     }
+    PerformedPhiTranslation |= UpwardDefsBegin.performedPhiTranslation();
   }
----------------
fhahn wrote:
> I think currently we only set `PerfomedPhiTranslation` to true if we preformed phi translation for the first visited upwards def (we only check the status for the iterator of the first def, which will be copied and only the copies will be updated). But we could also perform phi translation for other defs we visit in with the iterator. 
> 
> I think this is causing https://bugs.llvm.org/show_bug.cgi?id=47498 and I think we should set `PerfomedPhiTranslation` to true, if we do phi translations for any def visited in the iterator. I went ahead and pushed c4f1b3144184e4c276a7e7c801cbcd4ac3c573ba which changes the iterator to update a pointer to a bool to unblock a test-suite failure on PPC. I would appreciate if you could take a quick look to confirm this is indeed the expected behavior and I did not miss anything. Thanks!
You're right, this was missing the bools set by the internal iterations. Thank you for the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84905