[llvm] 80e8025 - [LV] Workaround PR49900 (a crash due to analyzing partially mutated IR)

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2021-05-05T09:56:28-07:00
New Revision: 80e8025083982f4eca8ca8200eafecf4a5c3ae6e

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

LOG: [LV] Workaround PR49900 (a crash due to analyzing partially mutated IR)

LoopVectorize has a fairly deeply baked in design problem where it will try to query analysis (primarily SCEV, but also ValueTracking) in the midst of mutating IR. In particular, the intermediate IR state does not represent the semantics of the original (or final) program.

Fixing this for real is hard, but all of the cases seen so far share a common symptom. In cases seen to date, the analysis being queried is the computation of the original loop's trip count. We can fix this particular instance of the issue by simply computing the trip count early, and caching it.

I want to be really clear that this is nothing but a workaround. It does nothing to fix the root issue, and at best, delays the time until we have to fix this for real. Florian and I have discussed an eventual solution in the review comments for https://reviews.llvm.org/D100663, but it's a lot of work.

Test taken from https://reviews.llvm.org/D100663.

Differential Revision: https://reviews.llvm.org/D101487

Added: 
    llvm/test/Transforms/LoopVectorize/scev-during-mutation.ll

Modified: 
    llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index b2740826718f..accf5ff2973d 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -3633,6 +3633,15 @@ BasicBlock *InnerLoopVectorizer::createVectorizedLoopSkeleton() {
   // Get the metadata of the original loop before it gets modified.
   MDNode *OrigLoopID = OrigLoop->getLoopID();
 
+  // Workaround!  Compute the trip count of the original loop and cache it
+  // before we start modifying the CFG.  This code has a systemic problem
+  // wherein it tries to run analysis over partially constructed IR; this is
+  // wrong, and not simply for SCEV.  The trip count of the original loop
+  // simply happens to be prone to hitting this in practice.  In theory, we
+  // can hit the same issue for any SCEV, or ValueTracking query done during
+  // mutation.  See PR49900.
+  getOrCreateTripCount(OrigLoop);
+
   // Create an empty vector loop, and prepare basic blocks for the runtime
   // checks.
   Loop *Lp = createVectorLoopSkeleton("");

diff  --git a/llvm/test/Transforms/LoopVectorize/scev-during-mutation.ll b/llvm/test/Transforms/LoopVectorize/scev-during-mutation.ll
new file mode 100644
index 000000000000..ed6864bd6b2e
--- /dev/null
+++ b/llvm/test/Transforms/LoopVectorize/scev-during-mutation.ll
@@ -0,0 +1,77 @@
+; RUN: opt -loop-vectorize -force-vector-width=2 -force-vector-interleave=2 -scev-verify-ir -S %s | FileCheck %s
+
+; Make sure SCEV is not queried while the IR is temporarily invalid. The tests
+; deliberately do not check for details of the vectorized IR, because that's
+; not the focus of the test.
+
+define void @pr49538() {
+; CHECK-LABEL: @pr49538
+; CHECK: vector.body:
+;
+entry:
+  br label %loop.0
+
+loop.0:
+  %iv.0 = phi i16 [ -1, %entry ], [ %iv.0.next, %loop.0.latch ]
+  br label %loop.1
+
+loop.1:
+  %iv.1 = phi i16 [ -1, %loop.0 ], [ %iv.1.next, %loop.1 ]
+  %iv.1.next = add nsw i16 %iv.1, 1
+  %i6 = icmp eq i16 %iv.1.next, %iv.0
+  br i1 %i6, label %loop.0.latch, label %loop.1
+
+loop.0.latch:
+  %i8 = phi i16 [ 1, %loop.1 ]
+  %iv.0.next = add nsw i16 %iv.0, 1
+  %ec.0 = icmp eq i16 %iv.0.next, %i8
+  br i1 %ec.0, label %exit, label %loop.0
+
+exit:
+  ret void
+}
+
+define void @pr49900(i32 %x, i64* %ptr) {
+; CHECK-LABEL: @pr49900
+; CHECK: vector.body{{.*}}:
+; CHECK: vector.body{{.*}}:
+;
+entry:
+  br label %loop.0
+
+loop.0:                                              ; preds = %bb2, %bb
+  %ec.0 = icmp slt i32 %x, 0
+  br i1 %ec.0, label %loop.0, label %loop.1.ph
+
+loop.1.ph:                                              ; preds = %bb2
+  br label %loop.1
+
+loop.1:                                             ; preds = %bb33, %bb5
+  %iv.1 = phi i32 [ 0, %loop.1.ph ], [ %iv.3.next, %loop.1.latch ]
+  br label %loop.2
+
+loop.2:
+  %iv.2 = phi i32 [ %iv.1, %loop.1 ], [ %iv.2.next, %loop.2 ]
+  %tmp54 = add i32 %iv.2, 12
+  %iv.2.next = add i32 %iv.2, 13
+  %ext = zext i32 %iv.2.next to i64
+  %tmp56 = add nuw nsw i64 %ext, 1
+  %C6 = icmp sle i32 %tmp54, 65536
+  br i1 %C6, label %loop.2, label %loop.3.ph
+
+loop.3.ph:
+  br label %loop.3
+
+loop.3:
+  %iv.3 = phi i32 [ %iv.2.next, %loop.3.ph ], [ %iv.3.next, %loop.3 ]
+  %iv.3.next = add i32 %iv.3 , 13
+  %C1 = icmp ult i32 %iv.3.next, 65536
+  br i1 %C1, label %loop.3, label %loop.1.latch
+
+loop.1.latch:
+  %ec = icmp ne i32 %iv.1, 9999
+  br i1 %ec, label %loop.1, label %exit
+
+exit:
+  ret void
+}