Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,13 @@ class VPInstruction : public VPRecipeWithIRFlags,
/// value for lane \p Lane.
Value *generatePerLane(VPTransformState &State, const VPLane &Lane);

#if !defined(NDEBUG)
/// Return the number of operands determined by the opcode of the
/// VPInstruction. Returns -1u if the number of operands cannot be determined
/// directly by the opcode.
static unsigned getNumOperandsForOpcode(unsigned Opcode);
#endif

public:
VPInstruction(unsigned Opcode, ArrayRef<VPValue *> Operands, DebugLoc DL = {},
const Twine &Name = "")
Expand Down
57 changes: 57 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,65 @@ VPInstruction::VPInstruction(unsigned Opcode, ArrayRef<VPValue *> Operands,
VPIRMetadata(), Opcode(Opcode), Name(Name.str()) {
assert(flagsValidForOpcode(getOpcode()) &&
"Set flags not supported for the provided opcode");
assert((getNumOperandsForOpcode(Opcode) == -1u ||
getNumOperandsForOpcode(Opcode) == getNumOperands()) &&
"number of operands does not match opcode");
}

#ifndef NDEBUG
unsigned VPInstruction::getNumOperandsForOpcode(unsigned Opcode) {
if (Instruction::isUnaryOp(Opcode) || Instruction::isCast(Opcode))
return 1;

if (Instruction::isBinaryOp(Opcode))
return 2;

switch (Opcode) {
case VPInstruction::StepVector:
return 0;
case Instruction::Alloca:
case Instruction::ExtractValue:
case Instruction::Freeze:
case Instruction::Load:
case VPInstruction::AnyOf:
case VPInstruction::BranchOnCond:
case VPInstruction::CalculateTripCountMinusVF:
case VPInstruction::CanonicalIVIncrementForPart:
case VPInstruction::ExplicitVectorLength:
case VPInstruction::ExtractLastElement:
case VPInstruction::ExtractPenultimateElement:
case VPInstruction::FirstActiveLane:
case VPInstruction::Not:
return 1;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done thanks

case Instruction::ICmp:
case Instruction::FCmp:
case Instruction::Store:
case VPInstruction::ActiveLaneMask:
case VPInstruction::BranchOnCount:
case VPInstruction::ComputeReductionResult:
case VPInstruction::FirstOrderRecurrenceSplice:
case VPInstruction::LogicalAnd:
case VPInstruction::PtrAdd:
case VPInstruction::WideIVStep:
return 2;
case Instruction::Select:
case VPInstruction::ComputeAnyOfResult:
case VPInstruction::ReductionStartVector:
return 3;
case VPInstruction::ComputeFindLastIVResult:
return 4;
case Instruction::Call:
case Instruction::GetElementPtr:
case Instruction::PHI:
case Instruction::Switch:
// Cannot determine the number of operands from the opcode.
return -1u;
}
llvm_unreachable("all cases should be handled above");
}
#endif

bool VPInstruction::doesGeneratePerAllLanes() const {
return Opcode == VPInstruction::PtrAdd && !vputils::onlyFirstLaneUsed(this);
}
Expand Down
39 changes: 22 additions & 17 deletions llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -706,13 +706,15 @@ TEST_F(VPBasicBlockTest, reassociateBlocks) {

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
TEST_F(VPBasicBlockTest, print) {
VPInstruction *TC = new VPInstruction(Instruction::Add, {});
VPInstruction *TC = new VPInstruction(Instruction::PHI, {});
Copy link
Collaborator

Choose a reason for hiding this comment

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

To escape num operands check?
Considered scalar, w/o operands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The tests here just check the various print implementations, the actual opcodes/operands don't matter much, other than matching the print checks.

There's a chicken-egg problem here, as we need to get a VPValue before calling getPlan.

VPlan &Plan = getPlan(TC);
IntegerType *Int32 = IntegerType::get(C, 32);
VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1));
VPBasicBlock *VPBB0 = Plan.getEntry();
VPBB0->appendRecipe(TC);

VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
VPInstruction *I2 = new VPInstruction(Instruction::Sub, {I1});
VPInstruction *I1 = new VPInstruction(Instruction::Add, {Val, Val});
VPInstruction *I2 = new VPInstruction(Instruction::Sub, {I1, Val});
VPInstruction *I3 = new VPInstruction(Instruction::Br, {I1, I2});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Independent: a branch has two VPInstruction operands?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this just creates artificial VPInstructions to test printing, the ocpodes don't really matter, other than checking.

We should probably tighten down which kinds of opcodes are supported for VPInstruction


VPBasicBlock *VPBB1 = Plan.createVPBasicBlock("");
Expand All @@ -722,7 +724,7 @@ TEST_F(VPBasicBlockTest, print) {
VPBB1->setName("bb1");

VPInstruction *I4 = new VPInstruction(Instruction::Mul, {I2, I1});
VPInstruction *I5 = new VPInstruction(Instruction::Ret, {I4});
VPInstruction *I5 = new VPInstruction(Instruction::Br, {I4});
Comment on lines -725 to +727
Copy link
Collaborator

Choose a reason for hiding this comment

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

Avoid checking return with a single operand? A VPInstruction with an Instruction opcode should retain the latter's number of operands, but that may require checking an underlying Instruction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, we currently never create VPInstruction for ret instructions in LV, so it seemed better to use FNeg as used above.

VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
VPBB2->appendRecipe(I4);
VPBB2->appendRecipe(I5);
Expand Down Expand Up @@ -752,22 +754,22 @@ edge [fontname=Courier, fontsize=30]
compound=true
N0 [label =
"preheader:\l" +
" EMIT vp\<%1\> = add \l" +
" EMIT-SCALAR vp\<%1\> = phi \l" +
"Successor(s): bb1\l"
]
N0 -> N1 [ label=""]
N1 [label =
"bb1:\l" +
" EMIT vp\<%2\> = add \l" +
" EMIT vp\<%3\> = sub vp\<%2\>\l" +
" EMIT vp\<%2\> = add ir\<1\>, ir\<1\>\l" +
" EMIT vp\<%3\> = sub vp\<%2\>, ir\<1\>\l" +
" EMIT br vp\<%2\>, vp\<%3\>\l" +
"Successor(s): bb2\l"
]
N1 -> N2 [ label=""]
N2 [label =
"bb2:\l" +
" EMIT vp\<%5\> = mul vp\<%3\>, vp\<%2\>\l" +
" EMIT ret vp\<%5\>\l" +
" EMIT br vp\<%5\>\l" +
"Successor(s): ir-bb\<scalar.header\>\l"
]
N2 -> N3 [ label=""]
Expand All @@ -780,8 +782,8 @@ compound=true
EXPECT_EQ(ExpectedStr, FullDump);

const char *ExpectedBlock1Str = R"(bb1:
EMIT vp<%2> = add
EMIT vp<%3> = sub vp<%2>
EMIT vp<%2> = add ir<1>, ir<1>
EMIT vp<%3> = sub vp<%2>, ir<1>
EMIT br vp<%2>, vp<%3>
Successor(s): bb2
)";
Expand All @@ -793,7 +795,7 @@ Successor(s): bb2
// Ensure that numbering is good when dumping the second block in isolation.
const char *ExpectedBlock2Str = R"(bb2:
EMIT vp<%5> = mul vp<%3>, vp<%2>
EMIT ret vp<%5>
EMIT br vp<%5>
Successor(s): ir-bb<scalar.header>
)";
std::string Block2Dump;
Expand Down Expand Up @@ -909,9 +911,12 @@ TEST_F(VPBasicBlockTest, cloneAndPrint) {
VPlan &Plan = getPlan(nullptr);
VPBasicBlock *VPBB0 = Plan.getEntry();

VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
VPInstruction *I2 = new VPInstruction(Instruction::Sub, {I1});
VPInstruction *I3 = new VPInstruction(Instruction::Br, {I1, I2});
IntegerType *Int32 = IntegerType::get(C, 32);
VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1));

VPInstruction *I1 = new VPInstruction(Instruction::Add, {Val, Val});
VPInstruction *I2 = new VPInstruction(Instruction::Sub, {I1, Val});
VPInstruction *I3 = new VPInstruction(Instruction::Store, {I1, I2});

VPBasicBlock *VPBB1 = Plan.createVPBasicBlock("");
VPBB1->appendRecipe(I1);
Expand All @@ -932,9 +937,9 @@ compound=true
N0 -> N1 [ label=""]
N1 [label =
"bb1:\l" +
" EMIT vp\<%1\> = add \l" +
" EMIT vp\<%2\> = sub vp\<%1\>\l" +
" EMIT br vp\<%1\>, vp\<%2\>\l" +
" EMIT vp\<%1\> = add ir\<1\>, ir\<1\>\l" +
" EMIT vp\<%2\> = sub vp\<%1\>, ir\<1\>\l" +
" EMIT store vp\<%1\>, vp\<%2\>\l" +
"No successors\l"
]
}
Expand Down