[PHITransAddr] Don't translate unreachable values
authorDavid Majnemer <david.majnemer@gmail.com>
Mon, 1 Jun 2015 00:15:08 +0000 (00:15 +0000)
committerDavid Majnemer <david.majnemer@gmail.com>
Mon, 1 Jun 2015 00:15:08 +0000 (00:15 +0000)
Unreachable values may use themselves in strange ways due to their
dominance property.  Attempting to translate through them can lead to
infinite recursion, crashing LLVM.  Instead, claim that we weren't able
to translate the value.

This fixes PR23096.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@238702 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Analysis/PHITransAddr.h
lib/Analysis/MemoryDependenceAnalysis.cpp
lib/Analysis/PHITransAddr.cpp
test/Transforms/GVN/unreachable_block_infinite_loop.ll

index 84bb9d8008b5c4457247cd4b57870270d5fd569a..cbdbb88f740711d4ad240425d8a7a7b9a6b5b5f5 100644 (file)
@@ -75,12 +75,12 @@ public:
   bool IsPotentiallyPHITranslatable() const;
   
   /// PHITranslateValue - PHI translate the current address up the CFG from
-  /// CurBB to Pred, updating our state to reflect any needed changes.  If the
-  /// dominator tree DT is non-null, the translated value must dominate
+  /// CurBB to Pred, updating our state to reflect any needed changes.  If
+  /// 'MustDominate' is true, the translated value must dominate
   /// PredBB.  This returns true on failure and sets Addr to null.
   bool PHITranslateValue(BasicBlock *CurBB, BasicBlock *PredBB,
-                         const DominatorTree *DT);
-  
+                         const DominatorTree *DT, bool MustDominate);
+
   /// PHITranslateWithInsertion - PHI translate this value into the specified
   /// predecessor block, inserting a computation of the value if it is
   /// unavailable.
index 3c1826a58e669878057a3ff81d45c5919bac7c77..ecc1c9bda9b9ccb5ea89d9495b4afe1f7f76c7f3 100644 (file)
@@ -1278,8 +1278,7 @@ getNonLocalPointerDepFromBB(Instruction *QueryInst,
       // Get the PHI translated pointer in this predecessor.  This can fail if
       // not translatable, in which case the getAddr() returns null.
       PHITransAddr &PredPointer = PredList.back().second;
-      PredPointer.PHITranslateValue(BB, Pred, nullptr);
-
+      PredPointer.PHITranslateValue(BB, Pred, DT, /*MustDominate=*/false);
       Value *PredPtrVal = PredPointer.getAddr();
 
       // Check to see if we have already visited this pred block with another
index 71e2cdcbb42d1bc4264190cec91b571c9fb2119b..633d6aaad35e919cb17a13ec74367ffe4f6bc45c 100644 (file)
@@ -315,21 +315,26 @@ Value *PHITransAddr::PHITranslateSubExpr(Value *V, BasicBlock *CurBB,
 
 
 /// PHITranslateValue - PHI translate the current address up the CFG from
-/// CurBB to Pred, updating our state to reflect any needed changes.  If the
-/// dominator tree DT is non-null, the translated value must dominate
+/// CurBB to Pred, updating our state to reflect any needed changes.  If
+/// 'MustDominate' is true, the translated value must dominate
 /// PredBB.  This returns true on failure and sets Addr to null.
 bool PHITransAddr::PHITranslateValue(BasicBlock *CurBB, BasicBlock *PredBB,
-                                     const DominatorTree *DT) {
+                                     const DominatorTree *DT,
+                                     bool MustDominate) {
+  assert(DT || !MustDominate);
   assert(Verify() && "Invalid PHITransAddr!");
-  Addr = PHITranslateSubExpr(Addr, CurBB, PredBB, DT);
+  if (DT && DT->isReachableFromEntry(PredBB))
+    Addr =
+        PHITranslateSubExpr(Addr, CurBB, PredBB, MustDominate ? DT : nullptr);
+  else
+    Addr = nullptr;
   assert(Verify() && "Invalid PHITransAddr!");
 
-  if (DT) {
+  if (MustDominate)
     // Make sure the value is live in the predecessor.
     if (Instruction *Inst = dyn_cast_or_null<Instruction>(Addr))
       if (!DT->dominates(Inst->getParent(), PredBB))
         Addr = nullptr;
-  }
 
   return Addr == nullptr;
 }
@@ -372,7 +377,7 @@ InsertPHITranslatedSubExpr(Value *InVal, BasicBlock *CurBB,
   // See if we have a version of this value already available and dominating
   // PredBB.  If so, there is no need to insert a new instance of it.
   PHITransAddr Tmp(InVal, DL, AC);
-  if (!Tmp.PHITranslateValue(CurBB, PredBB, &DT))
+  if (!Tmp.PHITranslateValue(CurBB, PredBB, &DT, /*MustDominate=*/true))
     return Tmp.getAddr();
 
   // If we don't have an available version of this value, it must be an
index fca5a28b38dd63b09023038bdefc2a49e12d6d51..a47e9e4c3a044b2bfc136fabc1266742b73ab08c 100644 (file)
@@ -12,3 +12,32 @@ unreachable_block:
     ret i32 %a
 }
 
+define i32 @pr23096_test0() {
+entry:
+  br label %bb0
+
+bb1:
+  %ptr1 = ptrtoint i32* %ptr2 to i64
+  %ptr2 = inttoptr i64 %ptr1 to i32*
+  br i1 undef, label %bb0, label %bb1
+
+bb0:
+  %phi = phi i32* [ undef, %entry ], [ %ptr2, %bb1 ]
+  %load = load i32, i32* %phi
+  ret i32 %load
+}
+
+define i32 @pr23096_test1() {
+entry:
+  br label %bb0
+
+bb1:
+  %ptr1 = getelementptr i32, i32* %ptr2, i32 0
+  %ptr2 = getelementptr i32, i32* %ptr1, i32 0
+  br i1 undef, label %bb0, label %bb1
+
+bb0:
+  %phi = phi i32* [ undef, %entry ], [ %ptr2, %bb1 ]
+  %load = load i32, i32* %phi
+  ret i32 %load
+}