[LV] Workaround PR49900

[reames]

Summary

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.

[fhahn]

LGTM, thanks! From the comment it's clear that this is jus a work-around, but as discussed in D100663, the general solution is non-trivial.

[fhahn]

If possible, it would be great if the title could be tweaked to be more expressive with respect to what the patch fixes/what it does.

[reames]

I seem to have forgotten to link the bug from the commit message. Logging it here for later reference.

https://bugs.llvm.org/show_bug.cgi?id=49900

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

|   [ ]_|   <-- old scalar loop to handle remainder.
      \  v
       >[ ]     <-- exit block.
    ...
    */
 
   // 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("");
 
   // Now, compare the new count to zero. If it is zero skip the vector loop and
   // jump to the scalar loop. This check also covers the case where the
   // backedge-taken count is uint##_max: adding one to it will overflow leading
   // to an incorrect trip count of zero. In this (rare) case we will also jump

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

+; 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
+}