[llvm] r226793 - Fix crashes in IRCE caused by mismatched types

Sanjoy Das sanjoy at playingwithpointers.com
Thu Jan 22 00:29:18 PST 2015


Author: sanjoy
Date: Thu Jan 22 02:29:18 2015
New Revision: 226793

URL: http://llvm.org/viewvc/llvm-project?rev=226793&view=rev
Log:
Fix crashes in IRCE caused by mismatched types

There are places where the inductive range check elimination pass
depends on two llvm::Values or llvm::SCEVs to be of the same
llvm::Type when they do not need to be. This patch relaxes those
restrictions (by bailing out of the optimization if the types
mismatch), and adds test cases to trigger those paths.

These issues were found by bootstrapping clang with IRCE running in
the -O3 pass ordering.

Differential Revision: http://reviews.llvm.org/D7082


Added:
    llvm/trunk/test/Transforms/IRCE/bug-mismatched-types.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp?rev=226793&r1=226792&r2=226793&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp Thu Jan 22 02:29:18 2015
@@ -489,8 +489,9 @@ class LoopConstrainer {
   // Compute a safe set of limits for the main loop to run in -- effectively the
   // intersection of `Range' and the iteration space of the original loop.
   // Return the header count (1 + the latch taken count) in `HeaderCount'.
+  // Return None if unable to compute the set of subranges.
   //
-  SubRanges calculateSubRanges(Value *&HeaderCount) const;
+  Optional<SubRanges> calculateSubRanges(Value *&HeaderCount) const;
 
   // Clone `OriginalLoop' and return the result in CLResult.  The IR after
   // running `cloneLoop' is well formed except for the PHI nodes in CLResult --
@@ -660,8 +661,14 @@ bool LoopConstrainer::recognizeLoop(Loop
     return false;
   }
 
+  // IndVarSimplify will sometimes leave behind (in SCEV's cache) backedge-taken
+  // counts that are narrower than the canonical induction variable.  These
+  // values are still accurate, and we could probably use them after sign/zero
+  // extension; but for now we just bail out of the transformation to keep
+  // things simple.
   const SCEV *CIVComparedToSCEV = SE.getSCEV(CIVComparedTo);
-  if (isa<SCEVCouldNotCompute>(CIVComparedToSCEV)) {
+  if (isa<SCEVCouldNotCompute>(CIVComparedToSCEV) ||
+      CIVComparedToSCEV->getType() != LatchCount->getType()) {
     FailureReason = "could not relate CIV to latch expression";
     return false;
   }
@@ -699,10 +706,15 @@ bool LoopConstrainer::recognizeLoop(Loop
   return true;
 }
 
-LoopConstrainer::SubRanges
+Optional<LoopConstrainer::SubRanges>
 LoopConstrainer::calculateSubRanges(Value *&HeaderCountOut) const {
   IntegerType *Ty = cast<IntegerType>(LatchTakenCount->getType());
 
+  assert(Range.first->getType() == Range.second->getType() &&
+         "ill-typed range!");
+  if (Range.first->getType() != Ty)
+    return None;
+
   SCEVExpander Expander(SE, "irce");
   Instruction *InsertPt = OriginalPreheader->getTerminator();
 
@@ -999,7 +1011,12 @@ bool LoopConstrainer::run() {
   OriginalPreheader = Preheader;
   MainLoopPreheader = Preheader;
 
-  SubRanges SR = calculateSubRanges(OriginalHeaderCount);
+  Optional<SubRanges> MaybeSR = calculateSubRanges(OriginalHeaderCount);
+  if (!MaybeSR.hasValue()) {
+    DEBUG(dbgs() << "irce: could not compute subranges\n");
+    return false;
+  }
+  SubRanges SR = MaybeSR.getValue();
 
   // It would have been better to make `PreLoop' and `PostLoop'
   // `Optional<ClonedLoop>'s, but `ValueToValueMapTy' does not have a copy
@@ -1113,13 +1130,20 @@ InductiveRangeCheck::computeSafeIteratio
   return std::make_pair(Begin, End);
 }
 
-static InductiveRangeCheck::Range
+static Optional<InductiveRangeCheck::Range>
 IntersectRange(const Optional<InductiveRangeCheck::Range> &R1,
                const InductiveRangeCheck::Range &R2, IRBuilder<> &B) {
+  assert(R2.first->getType() == R2.second->getType() && "ill-typed range!");
+
   if (!R1.hasValue())
     return R2;
   auto &R1Value = R1.getValue();
 
+  // TODO: we could widen the smaller range and have this work; but for now we
+  // bail out to keep things simple.
+  if (R1Value.first->getType() != R2.first->getType())
+    return None;
+
   Value *NewMin = ConstructSMaxOf(R1Value.first, R2.first, B);
   Value *NewMax = ConstructSMinOf(R1Value.second, R2.second, B);
   return std::make_pair(NewMin, NewMax);
@@ -1167,8 +1191,12 @@ bool InductiveRangeCheckElimination::run
   for (InductiveRangeCheck *IRC : RangeChecks) {
     auto Result = IRC->computeSafeIterationSpace(SE, B);
     if (Result.hasValue()) {
-      SafeIterRange = IntersectRange(SafeIterRange, Result.getValue(), B);
-      RangeChecksToEliminate.push_back(IRC);
+      auto MaybeSafeIterRange =
+        IntersectRange(SafeIterRange, Result.getValue(), B);
+      if (MaybeSafeIterRange.hasValue()) {
+        RangeChecksToEliminate.push_back(IRC);
+        SafeIterRange = MaybeSafeIterRange.getValue();
+      }
     }
   }
 

Added: llvm/trunk/test/Transforms/IRCE/bug-mismatched-types.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IRCE/bug-mismatched-types.ll?rev=226793&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/IRCE/bug-mismatched-types.ll (added)
+++ llvm/trunk/test/Transforms/IRCE/bug-mismatched-types.ll Thu Jan 22 02:29:18 2015
@@ -0,0 +1,66 @@
+; RUN: opt -irce -S < %s
+
+; These test cases don't check the correctness of the transform, but
+; that the -irce does not crash in the presence of certain things in
+; the IR:
+
+define void @mismatched_types_1() {
+; In this test case, the safe range for the only range check in the
+; loop is of type [i32, i32) while the backedge taken count is of type
+; i64.
+
+; CHECK-LABEL: mismatched_types_1
+entry:
+  br label %for.body
+
+for.body:
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.inc ]
+  %0 = trunc i64 %indvars.iv to i32
+  %1 = icmp ult i32 %0, 7
+  br i1 %1, label %switch.lookup, label %for.inc
+
+switch.lookup:
+  br label %for.inc
+
+for.inc:
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %cmp55 = icmp slt i64 %indvars.iv.next, 11
+  br i1 %cmp55, label %for.body, label %for.end
+
+for.end:
+  unreachable
+}
+
+define void @mismatched_types_2() {
+; In this test case, there are two range check in the loop, one with a
+; safe range of type [i32, i32) and one with a safe range of type
+; [i64, i64).
+
+; CHECK-LABEL: mismatched_types_2
+entry:
+  br label %for.body.a
+
+for.body.a:
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.inc ]
+  %cond.a = icmp ult i64 %indvars.iv, 7
+  br i1 %cond.a, label %switch.lookup.a, label %for.body.b
+
+switch.lookup.a:
+  br label %for.body.b
+
+for.body.b:
+  %truncated = trunc i64 %indvars.iv to i32
+  %cond.b = icmp ult i32 %truncated, 7
+  br i1 %cond.b, label %switch.lookup.b, label %for.inc
+
+switch.lookup.b:
+  br label %for.inc
+
+for.inc:
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %cmp55 = icmp slt i64 %indvars.iv.next, 11
+  br i1 %cmp55, label %for.body.a, label %for.end
+
+for.end:
+  unreachable
+}





More information about the llvm-commits mailing list