Skip to content

Conversation

@artagnon
Copy link
Contributor

@artagnon artagnon commented Nov 3, 2025

vputils::isSingleScalar is sufficient.

@llvmbot
Copy link
Member

llvmbot commented Nov 3, 2025

@llvm/pr-subscribers-vectorizers

Author: Ramkumar Ramachandra (artagnon)

Changes

The assert only currently holds by chance, as vputils::isSingleScalar handles a lot more recipes.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlan.cpp (-5)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 428a8f4c1348f..5b712d796496a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -308,11 +308,6 @@ Value *VPTransformState::get(const VPValue *Def, bool NeedsScalar) {
   VPLane LastLane(IsSingleScalar ? 0 : VF.getFixedValue() - 1);
   // Check if there is a scalar value for the selected lane.
   if (!hasScalarValue(Def, LastLane)) {
-    // At the moment, VPWidenIntOrFpInductionRecipes, VPScalarIVStepsRecipes and
-    // VPExpandSCEVRecipes can also be a single scalar.
-    assert((isa<VPWidenIntOrFpInductionRecipe, VPScalarIVStepsRecipe,
-                VPExpandSCEVRecipe>(Def->getDefiningRecipe())) &&
-           "unexpected recipe found to be invariant");
     IsSingleScalar = true;
     LastLane = 0;
   }

@llvmbot
Copy link
Member

llvmbot commented Nov 3, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

Changes

The assert only currently holds by chance, as vputils::isSingleScalar handles a lot more recipes.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlan.cpp (-5)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 428a8f4c1348f..5b712d796496a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -308,11 +308,6 @@ Value *VPTransformState::get(const VPValue *Def, bool NeedsScalar) {
   VPLane LastLane(IsSingleScalar ? 0 : VF.getFixedValue() - 1);
   // Check if there is a scalar value for the selected lane.
   if (!hasScalarValue(Def, LastLane)) {
-    // At the moment, VPWidenIntOrFpInductionRecipes, VPScalarIVStepsRecipes and
-    // VPExpandSCEVRecipes can also be a single scalar.
-    assert((isa<VPWidenIntOrFpInductionRecipe, VPScalarIVStepsRecipe,
-                VPExpandSCEVRecipe>(Def->getDefiningRecipe())) &&
-           "unexpected recipe found to be invariant");
     IsSingleScalar = true;
     LastLane = 0;
   }

The assert only currently holds by chance, as vputils::isSingleScalar
handles a lot more recipes.
@artagnon artagnon changed the title [VPlan] Strip bad assert in VPTransformState::get (NFC) [VPlan] Strip redundant code in VPTransformState::get (NFC) Nov 4, 2025
@artagnon artagnon force-pushed the vplan-transform-state-assert branch from 73c7ba2 to e8e57d5 Compare November 4, 2025 16:35
Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@artagnon artagnon merged commit c1ca4a5 into llvm:main Nov 5, 2025
10 checks passed
@artagnon artagnon deleted the vplan-transform-state-assert branch November 5, 2025 21:59
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