[VPlan] Thread scalar type through VPBlend, VPExpression recipes. (NFC)#200255
Merged
Conversation
Set the scalar type for VPBlendRecipe and VPExpressionRecipe at construction time, instead of inferring it on demand via VPTypeAnalysis. With this change, all VPValues have their scalar type set at construction, so VPTypeAnalysis::inferScalarType becomes a thin wrapper around VPValue::getScalarType. To be removed in a follow-up.
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesSet the scalar type for VPBlendRecipe and VPExpressionRecipe at construction time, instead of inferring it on demand via VPTypeAnalysis. To be removed in a follow-up. 5 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 78c55707b07eb..d07b9896514d7 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -2908,7 +2908,8 @@ class LLVM_ABI_FOR_TEST VPBlendRecipe : public VPRecipeWithIRFlags {
/// all other incoming values are merged into it.
VPBlendRecipe(PHINode *Phi, ArrayRef<VPValue *> Operands,
const VPIRFlags &Flags, DebugLoc DL)
- : VPRecipeWithIRFlags(VPRecipeBase::VPBlendSC, Operands, Flags, DL) {
+ : VPRecipeWithIRFlags(VPRecipeBase::VPBlendSC, Operands,
+ Operands[0]->getScalarType(), Flags, DL) {
assert(Operands.size() >= 2 && "Expected at least two operands!");
setUnderlyingValue(Phi);
}
@@ -3176,8 +3177,9 @@ class LLVM_ABI_FOR_TEST VPReductionRecipe : public VPRecipeWithIRFlags {
FastMathFlags FMFs, Instruction *I,
ArrayRef<VPValue *> Operands, VPValue *CondOp,
ReductionStyle Style, DebugLoc DL)
- : VPRecipeWithIRFlags(SC, Operands, FMFs, DL), RdxKind(RdxKind),
- Style(Style) {
+ : VPRecipeWithIRFlags(SC, Operands, Operands[0]->getScalarType(), FMFs,
+ DL),
+ RdxKind(RdxKind), Style(Style) {
if (CondOp) {
IsConditional = true;
addOperand(CondOp);
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index 16fd177ffc190..d6522bf702634 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -13,7 +13,6 @@
#include "VPlanHelpers.h"
#include "VPlanPatternMatch.h"
#include "llvm/ADT/PostOrderIterator.h"
-#include "llvm/ADT/TypeSwitch.h"
#include "llvm/Analysis/ScalarEvolution.h"
#include "llvm/Analysis/TargetTransformInfo.h"
#include "llvm/IR/Instruction.h"
@@ -24,47 +23,10 @@ using namespace VPlanPatternMatch;
#define DEBUG_TYPE "vplan"
-Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPBlendRecipe *R) {
- Type *ResTy = inferScalarType(R->getIncomingValue(0));
- for (unsigned I = 1, E = R->getNumIncomingValues(); I != E; ++I) {
- VPValue *Inc = R->getIncomingValue(I);
- assert(inferScalarType(Inc) == ResTy &&
- "different types inferred for different incoming values");
- CachedTypes[Inc] = ResTy;
- }
- return ResTy;
-}
-
Type *VPTypeAnalysis::inferScalarType(const VPValue *V) {
- if (Type *CachedTy = CachedTypes.lookup(V))
- return CachedTy;
-
- if (isa<VPIRValue, VPRegionValue, VPSymbolicValue, VPMultiDefValue,
- VPExpandSCEVRecipe, VPWidenPHIRecipe, VPPredInstPHIRecipe,
- VPScalarIVStepsRecipe, VPWidenCanonicalIVRecipe, VPWidenCastRecipe,
- VPWidenIntrinsicRecipe, VPWidenGEPRecipe, VPVectorPointerRecipe,
- VPVectorEndPointerRecipe, VPWidenCallRecipe, VPWidenLoadRecipe,
- VPWidenLoadEVLRecipe, VPDerivedIVRecipe, VPHeaderPHIRecipe,
- VPInstruction, VPReplicateRecipe, VPWidenRecipe>(V)) {
- Type *Ty = V->getScalarType();
- assert(Ty && "Scalar type must be set by recipe construction");
- return Ty;
- }
-
- Type *ResultTy =
- TypeSwitch<const VPRecipeBase *, Type *>(V->getDefiningRecipe())
- .Case<VPBlendRecipe>(
- [this](const auto *R) { return inferScalarTypeForRecipe(R); })
- .Case([this](const VPReductionRecipe *R) {
- return inferScalarType(R->getChainOp());
- })
- .Case([this](const VPExpressionRecipe *R) {
- return inferScalarType(R->getOperandOfResultType());
- });
-
- assert(ResultTy && "could not infer type for the given VPValue");
- CachedTypes[V] = ResultTy;
- return ResultTy;
+ Type *Ty = V->getScalarType();
+ assert(Ty && "Scalar type must be set by recipe construction");
+ return Ty;
}
void llvm::collectEphemeralRecipesForVPlan(
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.h b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.h
index 9c497ba110844..23feaf5615798 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.h
@@ -21,7 +21,6 @@ namespace llvm {
class LLVMContext;
class VPValue;
-class VPBlendRecipe;
class VPRecipeBase;
class VPlan;
class Value;
@@ -44,8 +43,6 @@ class VPTypeAnalysis {
LLVMContext &Ctx;
const DataLayout &DL;
- Type *inferScalarTypeForRecipe(const VPBlendRecipe *R);
-
public:
VPTypeAnalysis(const VPlan &Plan)
: Ctx(Plan.getContext()), DL(Plan.getDataLayout()) {}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index d841f54fa58a8..59084bbaf41b3 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3197,7 +3197,10 @@ InstructionCost VPReductionRecipe::computeCost(ElementCount VF,
VPExpressionRecipe::VPExpressionRecipe(
ExpressionTypes ExpressionType,
ArrayRef<VPSingleDefRecipe *> ExpressionRecipes)
- : VPSingleDefRecipe(VPRecipeBase::VPExpressionSC, {}),
+ : VPSingleDefRecipe(VPRecipeBase::VPExpressionSC, {},
+ cast<VPReductionRecipe>(ExpressionRecipes.back())
+ ->getChainOp()
+ ->getScalarType()),
ExpressionRecipes(ExpressionRecipes), ExpressionType(ExpressionType) {
assert(!ExpressionRecipes.empty() && "Nothing to combine?");
assert(
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index c0adc4d8c2050..276ef819be669 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -301,6 +301,10 @@ bool VPlanVerifier::verifyRecipeTypes(const VPRecipeBase &R) const {
return CheckScalarType(computeScalarTypeForInstruction(
cast<VPWidenRecipe>(&R)->getOpcode(), Ops));
}
+ case VPRecipeBase::VPExpressionSC:
+ return CheckScalarType(cast<VPExpressionRecipe>(&R)
+ ->getOperandOfResultType()
+ ->getScalarType());
default:
return true;
}
|
lukel97
approved these changes
May 29, 2026
| const VPIRFlags &Flags, DebugLoc DL) | ||
| : VPRecipeWithIRFlags(VPRecipeBase::VPBlendSC, Operands, Flags, DL) { | ||
| : VPRecipeWithIRFlags(VPRecipeBase::VPBlendSC, Operands, | ||
| Operands[0]->getScalarType(), Flags, DL) { |
Contributor
There was a problem hiding this comment.
Would be good to assert all operands have the same type at construction, but maybe something we can switch to doing for all recipes in a later PR
Contributor
Author
There was a problem hiding this comment.
Yep, I have a follow-up for that after landing this one and completing the migration.
For most recipes we already check the operands are valid for the provided opcode, but a few are still to cover. With that I think we can also remove the type verification in the verifier, as we ensure correctness at construction and modification (setOperand)
llvm-upstreamsync Bot
pushed a commit
to qualcomm/cpullvm-toolchain
that referenced
this pull request
May 30, 2026
…ecipes. (NFC) (#200255) Set the scalar type for VPBlendRecipe and VPExpressionRecipe at construction time, instead of inferring it on demand via VPTypeAnalysis. With this change, all VPValues have their scalar type set at construction, so VPTypeAnalysis::inferScalarType becomes a thin wrapper around VPValue::getScalarType. To be removed in a follow-up: llvm/llvm-project#200256. PR: llvm/llvm-project#200255
llvm-sync Bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 30, 2026
…ecipes. (NFC) (#200255) Set the scalar type for VPBlendRecipe and VPExpressionRecipe at construction time, instead of inferring it on demand via VPTypeAnalysis. With this change, all VPValues have their scalar type set at construction, so VPTypeAnalysis::inferScalarType becomes a thin wrapper around VPValue::getScalarType. To be removed in a follow-up: llvm/llvm-project#200256. PR: llvm/llvm-project#200255
fhahn
added a commit
that referenced
this pull request
Jun 2, 2026
…00256) Now that all VPValues have their scalar type set at construction, replace calls to VPTypeAnalysis::inferScalarType with direct calls to VPValue::getScalarType, and remove the no-longer-needed VPTypeAnalysis members from VPCostContext and VPTransformState. Also remove the getScalarTypeOrInfer fallback helper. Depends on #200255 PR: #200256
llvm-upstreamsync Bot
pushed a commit
to qualcomm/cpullvm-toolchain
that referenced
this pull request
Jun 2, 2026
…. (NFC) (#200256) Now that all VPValues have their scalar type set at construction, replace calls to VPTypeAnalysis::inferScalarType with direct calls to VPValue::getScalarType, and remove the no-longer-needed VPTypeAnalysis members from VPCostContext and VPTransformState. Also remove the getScalarTypeOrInfer fallback helper. Depends on llvm/llvm-project#200255 PR: llvm/llvm-project#200256
llvm-sync Bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 2, 2026
…. (NFC) (#200256) Now that all VPValues have their scalar type set at construction, replace calls to VPTypeAnalysis::inferScalarType with direct calls to VPValue::getScalarType, and remove the no-longer-needed VPTypeAnalysis members from VPCostContext and VPTransformState. Also remove the getScalarTypeOrInfer fallback helper. Depends on llvm/llvm-project#200255 PR: llvm/llvm-project#200256
yingopq
pushed a commit
to yingopq/llvm-project
that referenced
this pull request
Jun 5, 2026
…vm#200256) Now that all VPValues have their scalar type set at construction, replace calls to VPTypeAnalysis::inferScalarType with direct calls to VPValue::getScalarType, and remove the no-longer-needed VPTypeAnalysis members from VPCostContext and VPTransformState. Also remove the getScalarTypeOrInfer fallback helper. Depends on llvm#200255 PR: llvm#200256
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Set the scalar type for VPBlendRecipe and VPExpressionRecipe at construction time, instead of inferring it on demand via VPTypeAnalysis.
With this change, all VPValues have their scalar type set at construction, so VPTypeAnalysis::inferScalarType becomes a thin wrapper around VPValue::getScalarType.
To be removed in a follow-up: #200256.