[llvm] c4f1b31 - [MemorySSA] Make sure PerformedPhiTrans is updated for each visited def.

[Florian Hahn via llvm-commits]

Author: Florian Hahn
Date: 2020-09-14T16:11:56+01:00
New Revision: c4f1b3144184e4c276a7e7c801cbcd4ac3c573ba

URL: https://github.com/llvm/llvm-project/commit/c4f1b3144184e4c276a7e7c801cbcd4ac3c573ba
DIFF: https://github.com/llvm/llvm-project/commit/c4f1b3144184e4c276a7e7c801cbcd4ac3c573ba.diff

LOG: [MemorySSA] Make sure PerformedPhiTrans is updated for each visited def.

1ce82015f6d0 added a fix to restrict phi optimizations after phi
translations. But the current use of performedPhiTranslation only
checked whether phi translation happened for the first iterator and
missed cases where phi translations happens at subsequent
iterators/upwards defs.

This patch changes upward_defs_iteartor to take a pointer to a bool, so
we can easily ensure the final value includes all visited defs, while
still being able to conveniently use it with make_range & co.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/MemorySSA.h
    llvm/lib/Analysis/MemorySSA.cpp
    llvm/test/Analysis/MemorySSA/phi-translation.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/MemorySSA.h b/llvm/include/llvm/Analysis/MemorySSA.h
index 3ec09e8c0a45..5878b53fa372 100644
--- a/llvm/include/llvm/Analysis/MemorySSA.h
+++ b/llvm/include/llvm/Analysis/MemorySSA.h
@@ -1181,9 +1181,11 @@ class upward_defs_iterator
   using BaseT = upward_defs_iterator::iterator_facade_base;
 
 public:
-  upward_defs_iterator(const MemoryAccessPair &Info, DominatorTree *DT)
+  upward_defs_iterator(const MemoryAccessPair &Info, DominatorTree *DT,
+                       bool *PerformedPhiTranslation = nullptr)
       : DefIterator(Info.first), Location(Info.second),
-        OriginalAccess(Info.first), DT(DT) {
+        OriginalAccess(Info.first), DT(DT),
+        PerformedPhiTranslation(PerformedPhiTranslation) {
     CurrentPair.first = nullptr;
 
     WalkingPhi = Info.first && isa<MemoryPhi>(Info.first);
@@ -1214,8 +1216,6 @@ class upward_defs_iterator
 
   BasicBlock *getPhiArgBlock() const { return DefIterator.getPhiArgBlock(); }
 
-  bool performedPhiTranslation() const { return PerformedPhiTranslation; }
-
 private:
   void fillInCurrentPair() {
     CurrentPair.first = *DefIterator;
@@ -1228,7 +1228,8 @@ class upward_defs_iterator
                                         false)) {
         if (Translator.getAddr() != Location.Ptr) {
           CurrentPair.second = Location.getWithNewPtr(Translator.getAddr());
-          PerformedPhiTranslation = true;
+          if (PerformedPhiTranslation)
+            *PerformedPhiTranslation = true;
           return;
         }
       } else {
@@ -1245,12 +1246,13 @@ class upward_defs_iterator
   MemoryAccess *OriginalAccess = nullptr;
   DominatorTree *DT = nullptr;
   bool WalkingPhi = false;
-  bool PerformedPhiTranslation = false;
+  bool *PerformedPhiTranslation = nullptr;
 };
 
-inline upward_defs_iterator upward_defs_begin(const MemoryAccessPair &Pair,
-                                              DominatorTree &DT) {
-  return upward_defs_iterator(Pair, &DT);
+inline upward_defs_iterator
+upward_defs_begin(const MemoryAccessPair &Pair, DominatorTree &DT,
+                  bool *PerformedPhiTranslation = nullptr) {
+  return upward_defs_iterator(Pair, &DT, PerformedPhiTranslation);
 }
 
 inline upward_defs_iterator upward_defs_end() { return upward_defs_iterator(); }

diff  --git a/llvm/lib/Analysis/MemorySSA.cpp b/llvm/lib/Analysis/MemorySSA.cpp
index f54f04460a4d..14fa11988362 100644
--- a/llvm/lib/Analysis/MemorySSA.cpp
+++ b/llvm/lib/Analysis/MemorySSA.cpp
@@ -603,13 +603,13 @@ template <class AliasAnalysisType> class ClobberWalker {
 
   void addSearches(MemoryPhi *Phi, SmallVectorImpl<ListIndex> &PausedSearches,
                    ListIndex PriorNode) {
-    auto UpwardDefsBegin = upward_defs_begin({Phi, Paths[PriorNode].Loc}, DT);
+    auto UpwardDefsBegin = upward_defs_begin({Phi, Paths[PriorNode].Loc}, DT,
+                                             &PerformedPhiTranslation);
     auto UpwardDefs = make_range(UpwardDefsBegin, upward_defs_end());
     for (const MemoryAccessPair &P : UpwardDefs) {
       PausedSearches.push_back(Paths.size());
       Paths.emplace_back(P.second, P.first, PriorNode);
     }
-    PerformedPhiTranslation |= UpwardDefsBegin.performedPhiTranslation();
   }
 
   /// Represents a search that terminated after finding a clobber. This clobber

diff  --git a/llvm/test/Analysis/MemorySSA/phi-translation.ll b/llvm/test/Analysis/MemorySSA/phi-translation.ll
index 0844760327b1..1274e365066d 100644
--- a/llvm/test/Analysis/MemorySSA/phi-translation.ll
+++ b/llvm/test/Analysis/MemorySSA/phi-translation.ll
@@ -384,10 +384,9 @@ define void @dont_merge_noalias_complex_2(i32 %arg, i32 %arg1)  {
 ; CHECK-NEXT:  call void @init([32 x i32]* %tmp)
 
 ; CHECK-LABEL: loop.1.header:
-; CHECK-NEXT: ; 4 = MemoryPhi({entry,1},{loop.1.latch,3})
-; NOLIMIT:    ; MemoryUse(1) MayAlias
-; LIMIT:      ; MemoryUse(4) MayAlias
-; CHECK-NEXT: %l.1 = load i32, i32* %p.1, align 4
+; CHECK-NEXT:  ; 4 = MemoryPhi({entry,1},{loop.1.latch,3})
+; CHECK:       ; MemoryUse(4) MayAlias
+; CHECK-NEXT:  %l.1 = load i32, i32* %p.1, align 4
 
 ; CHECK-LABEL: loop.1.latch:
 ; CHECK-NEXT:  ; 3 = MemoryPhi({loop.1.header,4},{storebb,2})