[llvm] r248216 - Fix for pr24866

[Philip Reames via llvm-commits]

Author: reames
Date: Mon Sep 21 17:04:10 2015
New Revision: 248216

URL: http://llvm.org/viewvc/llvm-project?rev=248216&view=rev
Log:
Fix for pr24866

Turns out that not every basic block is guaranteed to have a node within the DominatorTree.  This is really hard to trigger, but the test case from the PR managed to do so.  There's active discussion continuing about what documentation and/or invariants needed cleaned up.


Added:
    llvm/trunk/test/Analysis/ValueTracking/pr24866.ll
Modified:
    llvm/trunk/lib/Analysis/ValueTracking.cpp

Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=248216&r1=248215&r2=248216&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp Mon Sep 21 17:04:10 2015
@@ -609,6 +609,11 @@ static void computeKnownBitsFromDominati
   if (!Q.DT || !Q.CxtI)
     return;
   Instruction *Cxt = const_cast<Instruction *>(Q.CxtI);
+  // The context instruction might be in a statically unreachable block.  If
+  // so, asking dominator queries may yield suprising results.  (e.g. the block
+  // may not have a dom tree node)
+  if (!Q.DT->isReachableFromEntry(Cxt->getParent()))
+    return;
 
   // Avoid useless work
   if (auto VI = dyn_cast<Instruction>(V))
@@ -655,7 +660,9 @@ static void computeKnownBitsFromDominati
     // instruction.  Finding a condition where one path dominates the context
     // isn't enough because both the true and false cases could merge before
     // the context instruction we're actually interested in.  Instead, we need
-    // to ensure that the taken *edge* dominates the context instruction.
+    // to ensure that the taken *edge* dominates the context instruction.  We
+    // know that the edge must be reachable since we started from a reachable
+    // block.
     BasicBlock *BB0 = BI->getSuccessor(0);
     BasicBlockEdge Edge(BI->getParent(), BB0);
     if (!Edge.isSingleEdge() || !Q.DT->dominates(Edge, Q.CxtI->getParent()))

Added: llvm/trunk/test/Analysis/ValueTracking/pr24866.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ValueTracking/pr24866.ll?rev=248216&view=auto
==============================================================================
--- llvm/trunk/test/Analysis/ValueTracking/pr24866.ll (added)
+++ llvm/trunk/test/Analysis/ValueTracking/pr24866.ll Mon Sep 21 17:04:10 2015
@@ -0,0 +1,44 @@
+; RUN: opt -S %s -value-tracking-dom-conditions -licm -load-combine | FileCheck %s
+; In pr24866.ll, we saw a crash when accessing a nullptr returned when
+; asking for a dominator tree Node.  This reproducer is really fragile,
+; but it's currently the best we have.
+
+%struct.c_derived_tbl.2.5.8.11.14.17.23.38.59.80.92.98.104.107.155.183 = type { [256 x i32], [256 x i8] }
+
+
+; Function Attrs: nounwind uwtable
+define void @encode_one_blockX2(%struct.c_derived_tbl.2.5.8.11.14.17.23.38.59.80.92.98.104.107.155.183* nocapture readonly %actbl) #0 {
+; CHECK-LABEL: @encode_one_blockX2
+entry:
+  br i1 false, label %L_KLOOP_01, label %L_KLOOP.preheader
+
+L_KLOOP_01:                                       ; preds = %while.end, %entry
+  br label %L_KLOOP.preheader
+
+L_KLOOP_08:                                       ; preds = %while.end
+  br label %L_KLOOP.preheader
+
+L_KLOOP.preheader:                                ; preds = %L_KLOOP_08, %L_KLOOP_01, %entry
+  %r.2.ph = phi i32 [ undef, %L_KLOOP_08 ], [ 0, %entry ], [ undef, %L_KLOOP_01 ]
+  br label %L_KLOOP
+
+L_KLOOP:                                          ; preds = %while.end, %L_KLOOP.preheader
+  %r.2 = phi i32 [ 0, %while.end ], [ %r.2.ph, %L_KLOOP.preheader ]
+  br i1 true, label %while.body, label %while.end
+
+while.body:                                       ; preds = %while.body, %L_KLOOP
+  br label %while.body
+
+while.end:                                        ; preds = %L_KLOOP
+  %shl105 = shl i32 %r.2, 4
+  %add106 = add nsw i32 %shl105, undef
+  %idxprom107 = sext i32 %add106 to i64
+  %arrayidx108 = getelementptr inbounds %struct.c_derived_tbl.2.5.8.11.14.17.23.38.59.80.92.98.104.107.155.183, %struct.c_derived_tbl.2.5.8.11.14.17.23.38.59.80.92.98.104.107.155.183* %actbl, i64 0, i32 0, i64 %idxprom107
+  %0 = load i32, i32* %arrayidx108, align 4
+  %arrayidx110 = getelementptr inbounds %struct.c_derived_tbl.2.5.8.11.14.17.23.38.59.80.92.98.104.107.155.183, %struct.c_derived_tbl.2.5.8.11.14.17.23.38.59.80.92.98.104.107.155.183* %actbl, i64 0, i32 1, i64 %idxprom107
+  %1 = load i8, i8* %arrayidx110, align 1
+  indirectbr i8* undef, [label %L_KLOOP_DONE, label %L_KLOOP_01, label %L_KLOOP_08, label %L_KLOOP]
+
+L_KLOOP_DONE:                                     ; preds = %while.end
+  ret void
+}