-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[VPlan] Introduce VPInstructionWithType, use instead of VPScalarCast(NFC) #129706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
29a14c1
060d094
3435b9f
e8623c4
55458d3
1280413
980a2f7
d9fb162
b0ddcb9
a2ec8d1
b08078a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4489,7 +4489,6 @@ static bool willGenerateVectors(VPlan &Plan, ElementCount VF, | |
| switch (R.getVPDefID()) { | ||
| case VPDef::VPDerivedIVSC: | ||
| case VPDef::VPScalarIVStepsSC: | ||
| case VPDef::VPScalarCastSC: | ||
| case VPDef::VPReplicateSC: | ||
| case VPDef::VPInstructionSC: | ||
| case VPDef::VPCanonicalIVPHISC: | ||
|
|
@@ -10424,8 +10423,9 @@ preparePlanForEpilogueVectorLoop(VPlan &Plan, Loop *L, | |
| assert(all_of(IV->users(), | ||
| [](const VPUser *U) { | ||
| return isa<VPScalarIVStepsRecipe>(U) || | ||
| isa<VPScalarCastRecipe>(U) || | ||
| isa<VPDerivedIVRecipe>(U) || | ||
| Instruction::isCast( | ||
| cast<VPInstruction>(U)->getOpcode()) || | ||
|
||
| cast<VPInstruction>(U)->getOpcode() == | ||
| Instruction::Add; | ||
| }) && | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -533,7 +533,6 @@ class VPSingleDefRecipe : public VPRecipeBase, public VPValue { | |||||
| case VPRecipeBase::VPWidenIntOrFpInductionSC: | ||||||
| case VPRecipeBase::VPWidenPointerInductionSC: | ||||||
| case VPRecipeBase::VPReductionPHISC: | ||||||
| case VPRecipeBase::VPScalarCastSC: | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. VPInstructionSC above now also holds for VPInstructionWithType recipes, instead of VPScalarCastSC. |
||||||
| case VPRecipeBase::VPScalarPHISC: | ||||||
| case VPRecipeBase::VPPartialReductionSC: | ||||||
| return true; | ||||||
|
|
@@ -1026,6 +1025,55 @@ class VPInstruction : public VPRecipeWithIRFlags, | |||||
| StringRef getName() const { return Name; } | ||||||
| }; | ||||||
|
|
||||||
| /// A specialization of VPInstruction augmenting it with a dedicated result | ||||||
| /// type, to be used when the opcode and operands of the VPInstruction don't | ||||||
| /// directly determine the result type. | ||||||
| class VPInstructionWithType : public VPInstruction { | ||||||
|
||||||
| /// Scalar result type produced by the recipe. | ||||||
| Type *ResultTy; | ||||||
|
|
||||||
| Value *generate(VPTransformState &State); | ||||||
|
||||||
|
|
||||||
| public: | ||||||
| VPInstructionWithType(unsigned Opcode, ArrayRef<VPValue *> Operands, | ||||||
| Type *ResultTy, DebugLoc DL, const Twine &Name = "") | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth documenting somewhere that VPInstructionWithType does not have its own VPDef::VPInstructionWithTypeSC identifying it, as do all other recipes, but it uses VPInstructionSC instead. VPInstructions should be identified according to their opcode, whether they are actually of class VPInstruction itself or a subclass thereof.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done thanks |
||||||
| : VPInstruction(Opcode, Operands, DL, Name), ResultTy(ResultTy) {} | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about FMF for, say, FPExt, or any other IRFlags supported by VPInstruction casts?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now, non of the cast take/use them, can be added as follow-up? |
||||||
|
|
||||||
| static inline bool classof(const VPRecipeBase *R) { | ||||||
| auto *VPI = dyn_cast<VPInstruction>(R); | ||||||
| return VPI && Instruction::isCast(VPI->getOpcode()); | ||||||
| } | ||||||
|
|
||||||
| static inline bool classof(const VPUser *R) { | ||||||
| return isa<VPInstructionWithType>(cast<VPRecipeBase>(R)); | ||||||
| } | ||||||
|
|
||||||
| VPInstruction *clone() override { | ||||||
| auto *New = | ||||||
| new VPInstructionWithType(getOpcode(), {getOperand(0)}, getResultType(), | ||||||
|
||||||
| getDebugLoc(), getName()); | ||||||
| New->setUnderlyingValue(getUnderlyingValue()); | ||||||
| return New; | ||||||
| } | ||||||
|
|
||||||
| void execute(VPTransformState &State) override; | ||||||
|
|
||||||
| /// Return the cost of this VPIRInstruction. | ||||||
|
||||||
| /// Return the cost of this VPIRInstruction. | |
| /// Return the cost of this VPInstruction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return actual cost here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now this only represents scalar casts, which are not corresponding to any input IR, hence are not yet costed (Added a TODO). It will be fleshed out in #129712.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,20 +252,17 @@ Type *VPTypeAnalysis::inferScalarType(const VPValue *V) { | |
| VPPartialReductionRecipe>([this](const VPRecipeBase *R) { | ||
| return inferScalarType(R->getOperand(0)); | ||
| }) | ||
| .Case<VPInstructionWithType, VPWidenIntrinsicRecipe>( | ||
| [](const auto *R) { return R->getResultType(); }) | ||
|
Comment on lines
+265
to
+266
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth noting that this case should appear before the next to catch VPInstructionWithType before its parent VPInstruction?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, thanks! |
||
| .Case<VPBlendRecipe, VPInstruction, VPWidenRecipe, VPReplicateRecipe, | ||
| VPWidenCallRecipe, VPWidenMemoryRecipe, VPWidenSelectRecipe>( | ||
| [this](const auto *R) { return inferScalarTypeForRecipe(R); }) | ||
| .Case<VPWidenIntrinsicRecipe>([](const VPWidenIntrinsicRecipe *R) { | ||
| return R->getResultType(); | ||
| }) | ||
| .Case<VPInterleaveRecipe>([V](const VPInterleaveRecipe *R) { | ||
| // TODO: Use info from interleave group. | ||
| return V->getUnderlyingValue()->getType(); | ||
| }) | ||
| .Case<VPWidenCastRecipe>( | ||
| [](const VPWidenCastRecipe *R) { return R->getResultType(); }) | ||
| .Case<VPScalarCastRecipe>( | ||
| [](const VPScalarCastRecipe *R) { return R->getResultType(); }) | ||
| .Case<VPExpandSCEVRecipe>([](const VPExpandSCEVRecipe *R) { | ||
| return R->getSCEV()->getType(); | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,7 +147,6 @@ bool VPRecipeBase::mayHaveSideEffects() const { | |
| switch (getVPDefID()) { | ||
| case VPDerivedIVSC: | ||
| case VPPredInstPHISC: | ||
| case VPScalarCastSC: | ||
lukel97 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| case VPReverseVectorPointerSC: | ||
| return false; | ||
| case VPInstructionSC: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which now takes care of VPInstructionWithType recipes as well, right?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep |
||
|
|
@@ -412,7 +411,7 @@ bool VPInstruction::doesGeneratePerAllLanes() const { | |
| } | ||
|
|
||
| bool VPInstruction::canGenerateScalarForFirstLane() const { | ||
| if (Instruction::isBinaryOp(getOpcode())) | ||
| if (Instruction::isBinaryOp(getOpcode()) || Instruction::isCast(getOpcode())) | ||
| return true; | ||
| if (isSingleScalar() || isVectorToScalar()) | ||
| return true; | ||
|
|
@@ -821,7 +820,7 @@ void VPInstruction::execute(VPTransformState &State) { | |
| } | ||
|
|
||
| bool VPInstruction::opcodeMayReadOrWriteFromMemory() const { | ||
| if (Instruction::isBinaryOp(getOpcode())) | ||
| if (Instruction::isBinaryOp(getOpcode()) || Instruction::isCast(getOpcode())) | ||
| return false; | ||
| switch (getOpcode()) { | ||
| case Instruction::ICmp: | ||
|
|
@@ -843,7 +842,7 @@ bool VPInstruction::opcodeMayReadOrWriteFromMemory() const { | |
|
|
||
| bool VPInstruction::onlyFirstLaneUsed(const VPValue *Op) const { | ||
| assert(is_contained(operands(), Op) && "Op must be an operand of the recipe"); | ||
| if (Instruction::isBinaryOp(getOpcode())) | ||
| if (Instruction::isBinaryOp(getOpcode()) || Instruction::isCast(getOpcode())) | ||
| return vputils::onlyFirstLaneUsed(this); | ||
|
|
||
| switch (getOpcode()) { | ||
|
|
@@ -972,6 +971,39 @@ void VPInstruction::print(raw_ostream &O, const Twine &Indent, | |
| } | ||
| #endif | ||
|
|
||
| Value *VPInstructionWithType::generate(VPTransformState &State) { | ||
| State.setDebugLocFrom(getDebugLoc()); | ||
| assert(vputils::onlyFirstLaneUsed(this) && | ||
| "Codegen only implemented for first lane."); | ||
| switch (getOpcode()) { | ||
| case Instruction::SExt: | ||
| case Instruction::ZExt: | ||
| case Instruction::Trunc: { | ||
| // Note: SExt/ZExt not used yet. | ||
|
||
| Value *Op = State.get(getOperand(0), VPLane(0)); | ||
| return State.Builder.CreateCast(Instruction::CastOps(getOpcode()), Op, | ||
| ResultTy); | ||
| } | ||
| default: | ||
| llvm_unreachable("opcode not implemented yet"); | ||
| } | ||
| } | ||
|
|
||
| void VPInstructionWithType::execute(VPTransformState &State) { | ||
| State.set(this, generate(State), VPLane(0)); | ||
| } | ||
|
|
||
| #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) | ||
| void VPInstructionWithType::print(raw_ostream &O, const Twine &Indent, | ||
| VPSlotTracker &SlotTracker) const { | ||
| O << Indent << "EMIT "; | ||
| printAsOperand(O, SlotTracker); | ||
| O << " = " << Instruction::getOpcodeName(getOpcode()) << " "; | ||
| printOperands(O, SlotTracker); | ||
| O << " to " << *ResultTy; | ||
| } | ||
| #endif | ||
|
|
||
| void VPIRInstruction::execute(VPTransformState &State) { | ||
| assert((isa<PHINode>(&I) || getNumOperands() == 0) && | ||
| "Only PHINodes can have extra operands"); | ||
|
|
@@ -2445,38 +2477,6 @@ void VPReplicateRecipe::print(raw_ostream &O, const Twine &Indent, | |
| } | ||
| #endif | ||
|
|
||
| Value *VPScalarCastRecipe ::generate(VPTransformState &State) { | ||
| State.setDebugLocFrom(getDebugLoc()); | ||
| assert(vputils::onlyFirstLaneUsed(this) && | ||
| "Codegen only implemented for first lane."); | ||
| switch (Opcode) { | ||
| case Instruction::SExt: | ||
| case Instruction::ZExt: | ||
| case Instruction::Trunc: { | ||
| // Note: SExt/ZExt not used yet. | ||
| Value *Op = State.get(getOperand(0), VPLane(0)); | ||
| return State.Builder.CreateCast(Instruction::CastOps(Opcode), Op, ResultTy); | ||
| } | ||
| default: | ||
| llvm_unreachable("opcode not implemented yet"); | ||
| } | ||
| } | ||
|
|
||
| void VPScalarCastRecipe ::execute(VPTransformState &State) { | ||
| State.set(this, generate(State), VPLane(0)); | ||
| } | ||
|
|
||
| #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) | ||
| void VPScalarCastRecipe ::print(raw_ostream &O, const Twine &Indent, | ||
| VPSlotTracker &SlotTracker) const { | ||
| O << Indent << "SCALAR-CAST "; | ||
| printAsOperand(O, SlotTracker); | ||
| O << " = " << Instruction::getOpcodeName(Opcode) << " "; | ||
| printOperands(O, SlotTracker); | ||
| O << " to " << *ResultTy; | ||
| } | ||
| #endif | ||
|
|
||
| void VPBranchOnMaskRecipe::execute(VPTransformState &State) { | ||
| State.setDebugLocFrom(getDebugLoc()); | ||
| assert(State.Lane && "Branch on Mask works only on single instance."); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -146,8 +146,8 @@ bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const { | |||||
| .Case<VPWidenLoadEVLRecipe, VPReverseVectorPointerRecipe, | ||||||
| VPScalarPHIRecipe>( | ||||||
| [&](const VPRecipeBase *R) { return VerifyEVLUse(*R, 1); }) | ||||||
| .Case<VPScalarCastRecipe>( | ||||||
| [&](const VPScalarCastRecipe *S) { return VerifyEVLUse(*S, 0); }) | ||||||
| .Case<VPInstructionWithType>( | ||||||
| [&](const auto *S) { return VerifyEVLUse(*S, 0); }) | ||||||
|
||||||
| [&](const auto *S) { return VerifyEVLUse(*S, 0); }) | |
| [&](const VPRecipeBase *S) { return VerifyEVLUse(*S, 0); }) |
consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to VPInstructionWithType,. thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VPInstructionSC now also holds for VPInstructionWithType recipes, instead of VPScalarCastSC.