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

[Florian Hahn via Phabricator via llvm-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0017c2bc258: [MemorySSA] Be more conservative when traversing MemoryPhis. (authored by fhahn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87778

Files:
  llvm/include/llvm/Analysis/MemorySSA.h
  llvm/test/Analysis/MemorySSA/phi-translation.ll


Index: llvm/test/Analysis/MemorySSA/phi-translation.ll
===================================================================
--- llvm/test/Analysis/MemorySSA/phi-translation.ll
+++ llvm/test/Analysis/MemorySSA/phi-translation.ll
@@ -481,8 +481,7 @@
 ; CHECK-NEXT:  ; 4 = MemoryPhi({entry,1},{cond.read,3})
 
 ; CHECK-LABEL: cond.read:
-; NOLIMIT:     ; MemoryUse(liveOnEntry)
-; LIMIT:       ; MemoryUse(4)
+; CHECK:       ; MemoryUse(4)
 ; CHECK-NEXT:  %use = load i32, i32* %ptr.1, align 4
 ; CHECK-NEXT:  ; 2 = MemoryDef(4)
 ; CHECK-NEXT:  %c.2 = call i1 @cond(i32 %use)
Index: llvm/include/llvm/Analysis/MemorySSA.h
===================================================================
--- llvm/include/llvm/Analysis/MemorySSA.h
+++ llvm/include/llvm/Analysis/MemorySSA.h
@@ -88,6 +88,7 @@
 #include "llvm/IR/DerivedUser.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/Operator.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Use.h"
 #include "llvm/IR/User.h"
@@ -1217,27 +1218,61 @@
   BasicBlock *getPhiArgBlock() const { return DefIterator.getPhiArgBlock(); }
 
 private:
+  /// Returns true if \p Ptr is guaranteed to be loop invariant for any possible
+  /// loop. In particular, this guarantees that it only references a single
+  /// MemoryLocation during execution of the containing function.
+  bool IsGuaranteedLoopInvariant(Value *Ptr) const {
+    auto IsGuaranteedLoopInvariantBase = [](Value *Ptr) {
+      Ptr = Ptr->stripPointerCasts();
+      if (auto *I = dyn_cast<Instruction>(Ptr)) {
+        if (isa<AllocaInst>(Ptr))
+          return true;
+        return false;
+      }
+      return true;
+    };
+
+    Ptr = Ptr->stripPointerCasts();
+    if (auto *GEP = dyn_cast<GEPOperator>(Ptr)) {
+      return IsGuaranteedLoopInvariantBase(GEP->getPointerOperand()) &&
+             GEP->hasAllConstantIndices();
+    }
+    return IsGuaranteedLoopInvariantBase(Ptr);
+  }
+
   void fillInCurrentPair() {
     CurrentPair.first = *DefIterator;
+    CurrentPair.second = Location;
     if (WalkingPhi && Location.Ptr) {
+      // Mark size as unknown, if the location is not guaranteed to be
+      // loop-invariant for any possible loop in the function. Setting the size
+      // to unknown guarantees that any memory accesses that access locations
+      // after the pointer are considered as clobbers, which is important to
+      // catch loop carried dependences.
+      if (Location.Ptr &&
+          !IsGuaranteedLoopInvariant(const_cast<Value *>(Location.Ptr)))
+        CurrentPair.second = Location.getWithNewSize(LocationSize::unknown());
       PHITransAddr Translator(
           const_cast<Value *>(Location.Ptr),
           OriginalAccess->getBlock()->getModule()->getDataLayout(), nullptr);
+
       if (!Translator.PHITranslateValue(OriginalAccess->getBlock(),
                                         DefIterator.getPhiArgBlock(), DT,
                                         true)) {
-        if (Translator.getAddr() != Location.Ptr) {
-          CurrentPair.second = Location.getWithNewPtr(Translator.getAddr());
+        Value *TransAddr = Translator.getAddr();
+        if (TransAddr != Location.Ptr) {
+          CurrentPair.second = CurrentPair.second.getWithNewPtr(TransAddr);
+
+          if (TransAddr &&
+              !IsGuaranteedLoopInvariant(const_cast<Value *>(TransAddr)))
+            CurrentPair.second =
+                CurrentPair.second.getWithNewSize(LocationSize::unknown());
+
           if (PerformedPhiTranslation)
             *PerformedPhiTranslation = true;
-          return;
         }
-      } else {
-        CurrentPair.second = Location.getWithNewSize(LocationSize::unknown());
-        return;
       }
     }
-    CurrentPair.second = Location;
   }
 
   MemoryAccessPair CurrentPair;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87778.292619.patch
Type: text/x-patch
Size: 3798 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200917/eb9bc09f/attachment.bin>