[llvm] [LV] Strip unmaintainable MinBWs assert (PR #136858)

Ramkumar Ramachandra via llvm-commits llvm-commits at lists.llvm.org
Sat May 3 05:43:43 PDT 2025


https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/136858

>From b4e2d02598fd3a0ab66aa30fc03c4c1737377c44 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Wed, 23 Apr 2025 13:38:52 +0100
Subject: [PATCH 1/3] [LV] Pre-commit test for #125278

---
 .../test/Transforms/LoopVectorize/pr125278.ll | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 llvm/test/Transforms/LoopVectorize/pr125278.ll

diff --git a/llvm/test/Transforms/LoopVectorize/pr125278.ll b/llvm/test/Transforms/LoopVectorize/pr125278.ll
new file mode 100644
index 0000000000000..5b81544848332
--- /dev/null
+++ b/llvm/test/Transforms/LoopVectorize/pr125278.ll
@@ -0,0 +1,21 @@
+; REQUIRES: asserts
+; RUN: not --crash opt -passes=loop-vectorize -force-vector-width=4 -disable-output %s
+
+define void @pr125278(ptr %dst, i64 %n) {
+entry:
+  %true.ext = zext i1 true to i32
+  br label %cond
+
+cond:
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 0, %cond ], [ %iv.next, %loop ]
+  %false.ext = zext i1 false to i32
+  %xor = xor i32 %false.ext, %true.ext
+  %xor.trunc = trunc i32 %xor to i8
+  store i8 %xor.trunc, ptr %dst, align 1
+  %iv.next = add i64 %iv, 1
+  %cmp = icmp ult i64 %iv.next, %n
+  br i1 %cmp, label %loop, label %cond
+}

>From 639ec9cbc0da03191b8f1bc7ba1a60e6b0499209 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Wed, 23 Apr 2025 13:42:00 +0100
Subject: [PATCH 2/3] [LV] Strip unmaintainable MinBWs assert

tryToWiden attempts to replace an Instruction with a Constant from SCEV,
but forgets to erase the Instruction from the MinBWs map, leading to an
assert in VPlanTransforms::truncateToMinimalBitwidths. Going forward,
the assertion in truncateToMinimalBitwidths is unmaintainable, as LV
could simplify the expression at any point: fix the bug by stripping the
unmaintable assertion.

Fixes #125278.
---
 .../Transforms/Vectorize/VPlanTransforms.cpp  | 17 --------
 .../test/Transforms/LoopVectorize/pr125278.ll | 41 ++++++++++++++++++-
 2 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index f2dc68b2ea8b6..f0473935ee552 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -1598,11 +1598,6 @@ static void licm(VPlan &Plan) {
 
 void VPlanTransforms::truncateToMinimalBitwidths(
     VPlan &Plan, const MapVector<Instruction *, uint64_t> &MinBWs) {
-#ifndef NDEBUG
-  // Count the processed recipes and cross check the count later with MinBWs
-  // size, to make sure all entries in MinBWs have been handled.
-  unsigned NumProcessedRecipes = 0;
-#endif
   // Keep track of created truncates, so they can be re-used. Note that we
   // cannot use RAUW after creating a new truncate, as this would could make
   // other uses have different types for their operands, making them invalidly
@@ -1624,9 +1619,6 @@ void VPlanTransforms::truncateToMinimalBitwidths(
       if (!NewResSizeInBits)
         continue;
 
-#ifndef NDEBUG
-      NumProcessedRecipes++;
-#endif
       // If the value wasn't vectorized, we must maintain the original scalar
       // type. Skip those here, after incrementing NumProcessedRecipes. Also
       // skip casts which do not need to be handled explicitly here, as
@@ -1650,7 +1642,6 @@ void VPlanTransforms::truncateToMinimalBitwidths(
             // Add an entry to ProcessedTruncs to avoid counting the same
             // operand multiple times.
             ProcessedTruncs[Op] = nullptr;
-            NumProcessedRecipes += 1;
           }
         }
 #endif
@@ -1714,19 +1705,11 @@ void VPlanTransforms::truncateToMinimalBitwidths(
           NewOp->insertBefore(&R);
         } else {
           PH->appendRecipe(NewOp);
-#ifndef NDEBUG
-          auto *OpInst = dyn_cast<Instruction>(Op->getLiveInIRValue());
-          bool IsContained = MinBWs.contains(OpInst);
-          NumProcessedRecipes += IsContained;
-#endif
         }
       }
 
     }
   }
-
-  assert(MinBWs.size() == NumProcessedRecipes &&
-         "some entries in MinBWs haven't been processed");
 }
 
 /// Remove BranchOnCond recipes with true conditions together with removing
diff --git a/llvm/test/Transforms/LoopVectorize/pr125278.ll b/llvm/test/Transforms/LoopVectorize/pr125278.ll
index 5b81544848332..2dc657ca447ab 100644
--- a/llvm/test/Transforms/LoopVectorize/pr125278.ll
+++ b/llvm/test/Transforms/LoopVectorize/pr125278.ll
@@ -1,7 +1,44 @@
-; REQUIRES: asserts
-; RUN: not --crash opt -passes=loop-vectorize -force-vector-width=4 -disable-output %s
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals none --version 5
+; RUN: opt -passes=loop-vectorize -force-vector-width=4 -S %s | FileCheck %s
 
 define void @pr125278(ptr %dst, i64 %n) {
+; CHECK-LABEL: define void @pr125278(
+; CHECK-SAME: ptr [[DST:%.*]], i64 [[N:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[TRUE_EXT:%.*]] = zext i1 true to i32
+; CHECK-NEXT:    [[UMAX:%.*]] = call i64 @llvm.umax.i64(i64 [[N]], i64 1)
+; CHECK-NEXT:    br label %[[COND:.*]]
+; CHECK:       [[COND_LOOPEXIT:.*]]:
+; CHECK-NEXT:    br label %[[COND]]
+; CHECK:       [[COND]]:
+; CHECK-NEXT:    [[MIN_ITERS_CHECK:%.*]] = icmp ult i64 [[UMAX]], 4
+; CHECK-NEXT:    br i1 [[MIN_ITERS_CHECK]], label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
+; CHECK:       [[VECTOR_PH]]:
+; CHECK-NEXT:    [[N_MOD_VF:%.*]] = urem i64 [[UMAX]], 4
+; CHECK-NEXT:    [[N_VEC:%.*]] = sub i64 [[UMAX]], [[N_MOD_VF]]
+; CHECK-NEXT:    br label %[[VECTOR_BODY:.*]]
+; CHECK:       [[VECTOR_BODY]]:
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], %[[VECTOR_BODY]] ]
+; CHECK-NEXT:    store i8 1, ptr [[DST]], align 1
+; CHECK-NEXT:    [[INDEX_NEXT]] = add nuw i64 [[INDEX]], 4
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
+; CHECK-NEXT:    br i1 [[TMP0]], label %[[MIDDLE_BLOCK:.*]], label %[[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
+; CHECK:       [[MIDDLE_BLOCK]]:
+; CHECK-NEXT:    [[CMP_N:%.*]] = icmp eq i64 [[UMAX]], [[N_VEC]]
+; CHECK-NEXT:    br i1 [[CMP_N]], label %[[COND_LOOPEXIT]], label %[[SCALAR_PH]]
+; CHECK:       [[SCALAR_PH]]:
+; CHECK-NEXT:    [[BC_RESUME_VAL:%.*]] = phi i64 [ [[N_VEC]], %[[MIDDLE_BLOCK]] ], [ 0, %[[COND]] ]
+; CHECK-NEXT:    br label %[[LOOP:.*]]
+; CHECK:       [[LOOP]]:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ [[BC_RESUME_VAL]], %[[SCALAR_PH]] ], [ [[IV_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT:    [[FALSE_EXT:%.*]] = zext i1 false to i32
+; CHECK-NEXT:    [[XOR:%.*]] = xor i32 [[FALSE_EXT]], [[TRUE_EXT]]
+; CHECK-NEXT:    [[XOR_TRUNC:%.*]] = trunc i32 [[XOR]] to i8
+; CHECK-NEXT:    store i8 [[XOR_TRUNC]], ptr [[DST]], align 1
+; CHECK-NEXT:    [[IV_NEXT]] = add i64 [[IV]], 1
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i64 [[IV_NEXT]], [[N]]
+; CHECK-NEXT:    br i1 [[CMP]], label %[[LOOP]], label %[[COND_LOOPEXIT]], !llvm.loop [[LOOP3:![0-9]+]]
+;
 entry:
   %true.ext = zext i1 true to i32
   br label %cond

>From 52c6be4342b2332437a1477d653bc67812e59a6d Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Sat, 3 May 2025 13:41:55 +0100
Subject: [PATCH 3/3] [VPlan] Strip entire NDEBUG section

---
 .../Transforms/Vectorize/VPlanTransforms.cpp  | 24 +------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index f0473935ee552..d60fc021a2a13 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -1623,30 +1623,8 @@ void VPlanTransforms::truncateToMinimalBitwidths(
       // type. Skip those here, after incrementing NumProcessedRecipes. Also
       // skip casts which do not need to be handled explicitly here, as
       // redundant casts will be removed during recipe simplification.
-      if (isa<VPReplicateRecipe, VPWidenCastRecipe>(&R)) {
-#ifndef NDEBUG
-        // If any of the operands is a live-in and not used by VPWidenRecipe or
-        // VPWidenSelectRecipe, but in MinBWs, make sure it is counted as
-        // processed as well. When MinBWs is currently constructed, there is no
-        // information about whether recipes are widened or replicated and in
-        // case they are reciplicated the operands are not truncated. Counting
-        // them them here ensures we do not miss any recipes in MinBWs.
-        // TODO: Remove once the analysis is done on VPlan.
-        for (VPValue *Op : R.operands()) {
-          if (!Op->isLiveIn())
-            continue;
-          auto *UV = dyn_cast_or_null<Instruction>(Op->getUnderlyingValue());
-          if (UV && MinBWs.contains(UV) && !ProcessedTruncs.contains(Op) &&
-              none_of(Op->users(),
-                      IsaPred<VPWidenRecipe, VPWidenSelectRecipe>)) {
-            // Add an entry to ProcessedTruncs to avoid counting the same
-            // operand multiple times.
-            ProcessedTruncs[Op] = nullptr;
-          }
-        }
-#endif
+      if (isa<VPReplicateRecipe, VPWidenCastRecipe>(&R))
         continue;
-      }
 
       Type *OldResTy = TypeInfo.inferScalarType(ResultVPV);
       unsigned OldResSizeInBits = OldResTy->getScalarSizeInBits();



More information about the llvm-commits mailing list