Skip to content

Conversation

@artagnon
Copy link
Contributor

CanUseVersionedStride is checking that the value is modeled in VPlan, but we can use replaceAllUsesWith unconditionally for this.

CanUseVersionedStride is checking that the value is modeled in VPlan,
but we can use replaceAllUsesWith unconditionally for this.
@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2025

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

Changes

CanUseVersionedStride is checking that the value is modeled in VPlan, but we can use replaceAllUsesWith unconditionally for this.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp (+2-9)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index e060e7081042a..cafddcc77d15d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -2909,13 +2909,6 @@ void VPlanTransforms::canonicalizeEVLLoops(VPlan &Plan) {
 void VPlanTransforms::replaceSymbolicStrides(
     VPlan &Plan, PredicatedScalarEvolution &PSE,
     const DenseMap<Value *, const SCEV *> &StridesMap) {
-  // Replace VPValues for known constant strides guaranteed by predicate scalar
-  // evolution.
-  auto CanUseVersionedStride = [&Plan](VPUser &U, unsigned) {
-    auto *R = cast<VPRecipeBase>(&U);
-    return R->getRegion() ||
-           R->getParent() == Plan.getVectorLoopRegion()->getSinglePredecessor();
-  };
   ValueToSCEVMapTy RewriteMap;
   for (const SCEV *Stride : StridesMap.values()) {
     using namespace SCEVPatternMatch;
@@ -2928,7 +2921,7 @@ void VPlanTransforms::replaceSymbolicStrides(
     auto *CI =
         Plan.getOrAddLiveIn(ConstantInt::get(Stride->getType(), *StrideConst));
     if (VPValue *StrideVPV = Plan.getLiveIn(StrideV))
-      StrideVPV->replaceUsesWithIf(CI, CanUseVersionedStride);
+      StrideVPV->replaceAllUsesWith(CI);
 
     // The versioned value may not be used in the loop directly but through a
     // sext/zext. Add new live-ins in those cases.
@@ -2942,7 +2935,7 @@ void VPlanTransforms::replaceSymbolicStrides(
       APInt C =
           isa<SExtInst>(U) ? StrideConst->sext(BW) : StrideConst->zext(BW);
       VPValue *CI = Plan.getOrAddLiveIn(ConstantInt::get(U->getType(), C));
-      StrideVPV->replaceUsesWithIf(CI, CanUseVersionedStride);
+      StrideVPV->replaceAllUsesWith(CI);
     }
     RewriteMap[StrideV] = PSE.getSCEV(StrideV);
   }

void VPlanTransforms::replaceSymbolicStrides(
VPlan &Plan, PredicatedScalarEvolution &PSE,
const DenseMap<Value *, const SCEV *> &StridesMap) {
// Replace VPValues for known constant strides guaranteed by predicate scalar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still holds I think, it make ssure we only replace users that are guaranteed to be dominated by the runtime checks we insert.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is the explanation for the check below, which I think should be kept to ensure we only replace strides we know will be guarded by runtime checks

@artagnon artagnon closed this Oct 22, 2025
@artagnon artagnon deleted the vplan-replacesymstrides-dead-code branch October 22, 2025 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants