[llvm] [VPlan] Strip erroneous poisonGuaranteesUB (PR #189374)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

<details>
<summary>Changes</summary>

A mysterious poisonGuaranteesUB was introduced in 2f7e218 ([VPlan] Add missing sext(sub) SCEV fold to getSCEVExprForVPValue) as part of a bugfix for handling of sext(sub nsw) in getSCEVExprForVPValue. Although the associated comment claims that the code mirrors SCEV's handling, the corresponding code in SCEV has no such poison-guarantees-UB checking. Moreover, poisonGuaranteesUB matches GEPs and casts non-exhaustively, and hence incorrectly. The added test case sext_sub_nsw_for_address only incidentally satisfies poisonGuaranteesUB, and stripping the routine has no impact on the test. Hence, strip the erroneous routine altogether.

---
Full diff: https://github.com/llvm/llvm-project/pull/189374.diff


2 Files Affected:

- (modified) llvm/lib/Transforms/Vectorize/VPlanUtils.cpp (+1-56) 
- (modified) llvm/test/Transforms/LoopVectorize/AArch64/induction-costs.ll (+1-2) 


``````````diff
diff --git a/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp b/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
index 60f931a524cbb..c9919c017fc62 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
@@ -83,61 +83,6 @@ bool vputils::isHeaderMask(const VPValue *V, const VPlan &Plan) {
          B == Plan.getBackedgeTakenCount();
 }
 
-/// Returns true if \p R propagates poison from any operand to its result.
-static bool propagatesPoisonFromRecipeOp(const VPRecipeBase *R) {
-  return TypeSwitch<const VPRecipeBase *, bool>(R)
-      .Case<VPWidenGEPRecipe, VPWidenCastRecipe>(
-          [](const VPRecipeBase *) { return true; })
-      .Case([](const VPReplicateRecipe *Rep) {
-        // GEP and casts propagate poison from all operands.
-        unsigned Opcode = Rep->getOpcode();
-        return Opcode == Instruction::GetElementPtr ||
-               Instruction::isCast(Opcode);
-      })
-      .Default([](const VPRecipeBase *) { return false; });
-}
-
-/// Returns true if \p V being poison is guaranteed to trigger UB because it
-/// propagates to the address of a memory recipe.
-static bool poisonGuaranteesUB(const VPValue *V) {
-  SmallPtrSet<const VPValue *, 8> Visited;
-  SmallVector<const VPValue *, 16> Worklist;
-
-  Worklist.push_back(V);
-
-  while (!Worklist.empty()) {
-    const VPValue *Current = Worklist.pop_back_val();
-    if (!Visited.insert(Current).second)
-      continue;
-
-    for (VPUser *U : Current->users()) {
-      // Check if Current is used as an address operand for load/store.
-      if (auto *MemR = dyn_cast<VPWidenMemoryRecipe>(U)) {
-        if (MemR->getAddr() == Current)
-          return true;
-        continue;
-      }
-      if (auto *Rep = dyn_cast<VPReplicateRecipe>(U)) {
-        unsigned Opcode = Rep->getOpcode();
-        if ((Opcode == Instruction::Load && Rep->getOperand(0) == Current) ||
-            (Opcode == Instruction::Store && Rep->getOperand(1) == Current))
-          return true;
-      }
-
-      // Check if poison propagates through this recipe to any of its users.
-      auto *R = cast<VPRecipeBase>(U);
-      for (const VPValue *Op : R->operands()) {
-        if (Op == Current && propagatesPoisonFromRecipeOp(R)) {
-          Worklist.push_back(R->getVPSingleValue());
-          break;
-        }
-      }
-    }
-  }
-
-  return false;
-}
-
 const SCEV *vputils::getSCEVExprForVPValue(const VPValue *V,
                                            PredicatedScalarEvolution &PSE,
                                            const Loop *L) {
@@ -217,7 +162,7 @@ const SCEV *vputils::getSCEVExprForVPValue(const VPValue *V,
     VPValue *SubLHS, *SubRHS;
     auto *SubR = dyn_cast<VPRecipeWithIRFlags>(LHSVal);
     if (match(LHSVal, m_Sub(m_VPValue(SubLHS), m_VPValue(SubRHS))) && SubR &&
-        SubR->hasNoSignedWrap() && poisonGuaranteesUB(LHSVal)) {
+        SubR->hasNoSignedWrap()) {
       const SCEV *V1 = getSCEVExprForVPValue(SubLHS, PSE, L);
       const SCEV *V2 = getSCEVExprForVPValue(SubRHS, PSE, L);
       if (!isa<SCEVCouldNotCompute>(V1) && !isa<SCEVCouldNotCompute>(V2))
diff --git a/llvm/test/Transforms/LoopVectorize/AArch64/induction-costs.ll b/llvm/test/Transforms/LoopVectorize/AArch64/induction-costs.ll
index 2e640e2d22658..e84abc749baa9 100644
--- a/llvm/test/Transforms/LoopVectorize/AArch64/induction-costs.ll
+++ b/llvm/test/Transforms/LoopVectorize/AArch64/induction-costs.ll
@@ -516,8 +516,7 @@ exit:
 
 ; Test that sext(sub nsw) used in address computation is handled correctly
 ; in VPlan cost model (must match SCEV's handling).
-define void at sext_sub_nsw_for_address(ptr %base, i64 %n, ptr %src) #0 {
-;
+define void @sext_sub_nsw_for_address(ptr %base, i64 %n, ptr %src) #0 {
 ; CHECK-LABEL: define void @sext_sub_nsw_for_address(
 ; CHECK-SAME: ptr [[BASE:%.*]], i64 [[N:%.*]], ptr [[SRC:%.*]]) #[[ATTR0:[0-9]+]] {
 ; CHECK-NEXT:  [[ITER_CHECK:.*]]:

``````````

</details>


https://github.com/llvm/llvm-project/pull/189374