Skip to content

[VPlan] Manage instruction metadata in VPlan. #135272

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Apr 24, 2025
Merged

[VPlan] Manage instruction metadata in VPlan. #135272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3760c4f
[VPlan] Manage instruction medata in VPlan.
fhahn Apr 9, 2025
d3edd89
!fixup address latest comments, thanks
fhahn Apr 19, 2025
980a939
Merge remote-tracking branch 'origin/main' into vplan-irmetadata
fhahn Apr 21, 2025
7ad19f7
!fixup address latest comments, thanks
fhahn Apr 21, 2025
f9d1fb5
Merge remote-tracking branch 'origin/main' into vplan-irmetadata
fhahn Apr 21, 2025
6c8ef1f
Merge remote-tracking branch 'origin/main' into vplan-irmetadata
fhahn Apr 22, 2025
ef9bcc9
!fixup address latest comments, thanks
fhahn Apr 22, 2025
681bb63
Merge remote-tracking branch 'origin/main' into vplan-irmetadata
fhahn Apr 22, 2025
14ee2b1
!fixup address latest comments, thanks!
fhahn Apr 22, 2025
923336d
Merge remote-tracking branch 'origin/main' into vplan-irmetadata
fhahn Apr 24, 2025
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
11 changes: 0 additions & 11 deletions llvm/lib/Transforms/Vectorize/VPlan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,17 +363,6 @@ void VPTransformState::addNewMetadata(Instruction *To,
LVer->annotateInstWithNoAlias(To, Orig);
}

void VPTransformState::addMetadata(Value *To, Instruction *From) {
// No source instruction to transfer metadata from?
if (!From)
return;

if (Instruction *ToI = dyn_cast<Instruction>(To)) {
propagateMetadata(ToI, From);
addNewMetadata(ToI, From);
}
}

void VPTransformState::setDebugLocFrom(DebugLoc DL) {
const DILocation *DIL = DL;
// When a FSDiscriminator is enabled, we don't need to add the multiply
Expand Down
50 changes: 33 additions & 17 deletions llvm/lib/Transforms/Vectorize/VPlan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1190,18 +1190,33 @@ struct VPIRPhi : public VPIRInstruction {
#endif
};

/// Helper to manage IR metadata for recipes. It filters out metadata that
/// cannot be propagated.
class VPIRMetadata {
SmallVector<std::pair<unsigned, MDNode *>> Metadata;

protected:
VPIRMetadata() {}
VPIRMetadata(Instruction &I) { getMetadataToPropagate(&I, Metadata); }

public:
/// Add all metadata to \p I.
void applyMetadata(Instruction &I) const;
};

/// VPWidenRecipe is a recipe for producing a widened instruction using the
/// opcode and operands of the recipe. This recipe covers most of the
/// traditional vectorization cases where each recipe transforms into a
/// vectorized version of itself.
class VPWidenRecipe : public VPRecipeWithIRFlags {
class VPWidenRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
unsigned Opcode;

protected:
template <typename IterT>
VPWidenRecipe(unsigned VPDefOpcode, Instruction &I,
iterator_range<IterT> Operands)
: VPRecipeWithIRFlags(VPDefOpcode, Operands, I), Opcode(I.getOpcode()) {}
: VPRecipeWithIRFlags(VPDefOpcode, Operands, I), VPIRMetadata(I),
Opcode(I.getOpcode()) {}

public:
template <typename IterT>
Expand Down Expand Up @@ -1236,7 +1251,7 @@ class VPWidenRecipe : public VPRecipeWithIRFlags {
};

/// VPWidenCastRecipe is a recipe to create vector cast instructions.
class VPWidenCastRecipe : public VPRecipeWithIRFlags {
class VPWidenCastRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
/// Cast instruction opcode.
Instruction::CastOps Opcode;

Expand All @@ -1246,15 +1261,15 @@ class VPWidenCastRecipe : public VPRecipeWithIRFlags {
public:
VPWidenCastRecipe(Instruction::CastOps Opcode, VPValue *Op, Type *ResultTy,
CastInst &UI)
: VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op, UI), Opcode(Opcode),
ResultTy(ResultTy) {
: VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op, UI), VPIRMetadata(UI),
Opcode(Opcode), ResultTy(ResultTy) {
assert(UI.getOpcode() == Opcode &&
"opcode of underlying cast doesn't match");
}

VPWidenCastRecipe(Instruction::CastOps Opcode, VPValue *Op, Type *ResultTy)
: VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op), Opcode(Opcode),
ResultTy(ResultTy) {}
: VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op), VPIRMetadata(),
Opcode(Opcode), ResultTy(ResultTy) {}

~VPWidenCastRecipe() override = default;

Expand Down Expand Up @@ -1288,7 +1303,7 @@ class VPWidenCastRecipe : public VPRecipeWithIRFlags {
};

/// A recipe for widening vector intrinsics.
class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
/// ID of the vector intrinsic to widen.
Intrinsic::ID VectorIntrinsicID;

Expand All @@ -1309,7 +1324,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
ArrayRef<VPValue *> CallArguments, Type *Ty,
DebugLoc DL = {})
: VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments, CI),
VectorIntrinsicID(VectorIntrinsicID), ResultTy(Ty),
VPIRMetadata(CI), VectorIntrinsicID(VectorIntrinsicID), ResultTy(Ty),
MayReadFromMemory(CI.mayReadFromMemory()),
MayWriteToMemory(CI.mayWriteToMemory()),
MayHaveSideEffects(CI.mayHaveSideEffects()) {}
Expand All @@ -1318,7 +1333,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
ArrayRef<VPValue *> CallArguments, Type *Ty,
DebugLoc DL = {})
: VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments, DL),
VectorIntrinsicID(VectorIntrinsicID), ResultTy(Ty) {
VPIRMetadata(), VectorIntrinsicID(VectorIntrinsicID), ResultTy(Ty) {
LLVMContext &Ctx = Ty->getContext();
AttributeSet Attrs = Intrinsic::getFnAttributes(Ctx, VectorIntrinsicID);
MemoryEffects ME = Attrs.getMemoryEffects();
Expand Down Expand Up @@ -1374,7 +1389,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
};

/// A recipe for widening Call instructions using library calls.
class VPWidenCallRecipe : public VPRecipeWithIRFlags {
class VPWidenCallRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
/// Variant stores a pointer to the chosen function. There is a 1:1 mapping
/// between a given VF and the chosen vectorized variant, so there will be a
/// different VPlan for each VF with a valid variant.
Expand All @@ -1385,7 +1400,7 @@ class VPWidenCallRecipe : public VPRecipeWithIRFlags {
ArrayRef<VPValue *> CallArguments, DebugLoc DL = {})
: VPRecipeWithIRFlags(VPDef::VPWidenCallSC, CallArguments,
*cast<Instruction>(UV)),
Variant(Variant) {
VPIRMetadata(*cast<Instruction>(UV)), Variant(Variant) {
assert(
isa<Function>(getOperand(getNumOperands() - 1)->getLiveInIRValue()) &&
"last operand must be the called function");
Expand Down Expand Up @@ -1471,10 +1486,11 @@ class VPHistogramRecipe : public VPRecipeBase {
};

/// A recipe for widening select instructions.
struct VPWidenSelectRecipe : public VPRecipeWithIRFlags {
struct VPWidenSelectRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
template <typename IterT>
VPWidenSelectRecipe(SelectInst &I, iterator_range<IterT> Operands)
: VPRecipeWithIRFlags(VPDef::VPWidenSelectSC, Operands, I) {}
: VPRecipeWithIRFlags(VPDef::VPWidenSelectSC, Operands, I),
VPIRMetadata(I) {}

~VPWidenSelectRecipe() override = default;

Expand Down Expand Up @@ -2602,7 +2618,7 @@ class VPPredInstPHIRecipe : public VPSingleDefRecipe {

/// A common base class for widening memory operations. An optional mask can be
/// provided as the last operand.
class VPWidenMemoryRecipe : public VPRecipeBase {
class VPWidenMemoryRecipe : public VPRecipeBase, public VPIRMetadata {
protected:
Instruction &Ingredient;

Expand All @@ -2626,8 +2642,8 @@ class VPWidenMemoryRecipe : public VPRecipeBase {
VPWidenMemoryRecipe(const char unsigned SC, Instruction &I,
std::initializer_list<VPValue *> Operands,
bool Consecutive, bool Reverse, DebugLoc DL)
: VPRecipeBase(SC, Operands, DL), Ingredient(I), Consecutive(Consecutive),
Reverse(Reverse) {
: VPRecipeBase(SC, Operands, DL), VPIRMetadata(I), Ingredient(I),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this way of initializing VPIRMetadata(I) be applied to all recipe constructors that receive their underlying instruction I as parameter? This seems like a natural default behavior, could be followed by an update if some other exceptional metadata is desired.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated for now, although it might be good to encourage avoidance of relying on underlying instructions on construction ;)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Certainly, by all means!
Any recipe that can be constructed w/o an underlying instruction should be; at that point its metadata can be passed instead of the instruction itself.

Consecutive(Consecutive), Reverse(Reverse) {
assert((Consecutive || !Reverse) && "Reverse implies consecutive");
}

Expand Down
7 changes: 0 additions & 7 deletions llvm/lib/Transforms/Vectorize/VPlanHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,6 @@ struct VPTransformState {
/// vector loop.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the above description still accurate? Would be good to clarify when State.addNewMetadata(To, Orig) needs to complement applyMetadata(To). Will possibly be handled by #136450

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is still accurate; applyMetadata in this patch applies metadata already on Orig, while addNewMetadata adds metadata from versioning, not present at the original instructions.

void addNewMetadata(Instruction *To, const Instruction *Orig);

/// Add metadata from one instruction to another.
///
/// This includes both the original MDs from \p From and additional ones (\see
/// addNewMetadata). Use this for *newly created* instructions in the vector
/// loop.
void addMetadata(Value *To, Instruction *From);

/// Set the debug location in the builder using the debug location \p DL.
void setDebugLocFrom(DebugLoc DL);

Expand Down
45 changes: 30 additions & 15 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1205,6 +1205,11 @@ void VPIRPhi::print(raw_ostream &O, const Twine &Indent,
}
#endif

void VPIRMetadata::applyMetadata(Instruction &I) const {
for (const auto &[Kind, Node] : Metadata)
I.setMetadata(Kind, Node);
}

void VPWidenCallRecipe::execute(VPTransformState &State) {
assert(State.VF.isVector() && "not widening");
assert(Variant != nullptr && "Can't create vector function.");
Expand All @@ -1231,11 +1236,11 @@ void VPWidenCallRecipe::execute(VPTransformState &State) {

CallInst *V = State.Builder.CreateCall(Variant, Args, OpBundles);
applyFlags(*V);
applyMetadata(*V);
V->setCallingConv(Variant->getCallingConv());

if (!V->getType()->isVoidTy())
State.set(this, V);
Comment on lines 1241 to 1243
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: good to place setMetadata() next to setFlags() in general, here and below.

Suggested change
if (!V->getType()->isVoidTy())
State.set(this, V);
setMetadata(V);
if (!V->getType()->isVoidTy())
State.set(this, V);

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

State.addMetadata(V, CI);
}

InstructionCost VPWidenCallRecipe::computeCost(ElementCount VF,
Expand Down Expand Up @@ -1311,10 +1316,10 @@ void VPWidenIntrinsicRecipe::execute(VPTransformState &State) {
CallInst *V = State.Builder.CreateCall(VectorF, Args, OpBundles);

applyFlags(*V);
applyMetadata(*V);

if (!V->getType()->isVoidTy())
State.set(this, V);
State.addMetadata(V, CI);
}

InstructionCost VPWidenIntrinsicRecipe::computeCost(ElementCount VF,
Expand Down Expand Up @@ -1509,9 +1514,11 @@ void VPWidenSelectRecipe::execute(VPTransformState &State) {
Value *Op1 = State.get(getOperand(2));
Value *Sel = State.Builder.CreateSelect(Cond, Op0, Op1);
State.set(this, Sel);
if (isa<FPMathOperator>(Sel))
applyFlags(*cast<Instruction>(Sel));
State.addMetadata(Sel, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
if (auto *I = dyn_cast<Instruction>(Sel)) {
if (isa<FPMathOperator>(I))
applyFlags(*I);
applyMetadata(*I);
}
Comment on lines +1517 to +1521
Copy link
Collaborator

Choose a reason for hiding this comment

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

Independent, consistency nit: good to keep State.set() as the last operation of execute(), after fully finishing to build the new instruction/value, including applying its flags/metadata. Conceptually, flags and metadata could be folded into Builder.CreateSelect().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do separately, thanks.

}

InstructionCost VPWidenSelectRecipe::computeCost(ElementCount VF,
Expand Down Expand Up @@ -1642,12 +1649,13 @@ void VPWidenRecipe::execute(VPTransformState &State) {

Value *V = Builder.CreateNAryOp(Opcode, Ops);

if (auto *VecOp = dyn_cast<Instruction>(V))
if (auto *VecOp = dyn_cast<Instruction>(V)) {
applyFlags(*VecOp);
applyMetadata(*VecOp);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here setFlags() is already applied under dyn_cast to Instruction. Place setMetadata() here as well, or rather applyMetadata(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.

Done, thanks

// Use this vector value for all users of the original instruction.
State.set(this, V);
State.addMetadata(V, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
break;
}
case Instruction::ExtractValue: {
Expand Down Expand Up @@ -1679,8 +1687,9 @@ void VPWidenRecipe::execute(VPTransformState &State) {
} else {
C = Builder.CreateICmp(getPredicate(), A, B);
}
if (auto *I = dyn_cast<Instruction>(C))
applyMetadata(*I);
State.set(this, C);
State.addMetadata(C, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
break;
}
default:
Expand Down Expand Up @@ -1796,9 +1805,10 @@ void VPWidenCastRecipe::execute(VPTransformState &State) {
Value *A = State.get(Op);
Value *Cast = Builder.CreateCast(Instruction::CastOps(Opcode), A, DestTy);
State.set(this, Cast);
State.addMetadata(Cast, cast_or_null<Instruction>(getUnderlyingValue()));
if (auto *CastOp = dyn_cast<Instruction>(Cast))
if (auto *CastOp = dyn_cast<Instruction>(Cast)) {
applyFlags(*CastOp);
applyMetadata(*CastOp);
}
Comment on lines -1800 to +1811
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same independent consistency nit.

}

InstructionCost VPWidenCastRecipe::computeCost(ElementCount VF,
Expand Down Expand Up @@ -2750,8 +2760,10 @@ void VPWidenLoadRecipe::execute(VPTransformState &State) {
} else {
NewLI = Builder.CreateAlignedLoad(DataTy, Addr, Alignment, "wide.load");
}
// Add metadata to the load, but setVectorValue to the reverse shuffle.
State.addMetadata(NewLI, cast<LoadInst>(&Ingredient));
// Add metadata to the load, but set the result to the reverse shuffle, if
// needed.
State.addNewMetadata(cast<Instruction>(NewLI), &Ingredient);
applyMetadata(*cast<Instruction>(NewLI));
if (Reverse)
NewLI = Builder.CreateVectorReverse(NewLI, "reverse");
State.set(this, NewLI);
Expand Down Expand Up @@ -2810,7 +2822,8 @@ void VPWidenLoadEVLRecipe::execute(VPTransformState &State) {
}
NewLI->addParamAttr(
0, Attribute::getWithAlignment(NewLI->getContext(), Alignment));
State.addMetadata(NewLI, cast<LoadInst>(&Ingredient));
State.addNewMetadata(NewLI, &Ingredient);
applyMetadata(*NewLI);
Instruction *Res = NewLI;
if (isReverse())
Res = createReverseEVL(Builder, Res, EVL, "vp.reverse");
Expand Down Expand Up @@ -2884,7 +2897,8 @@ void VPWidenStoreRecipe::execute(VPTransformState &State) {
NewSI = Builder.CreateMaskedStore(StoredVal, Addr, Alignment, Mask);
else
NewSI = Builder.CreateAlignedStore(StoredVal, Addr, Alignment);
State.addMetadata(NewSI, cast<StoreInst>(&Ingredient));
State.addNewMetadata(NewSI, &Ingredient);
applyMetadata(*NewSI);
}

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
Expand Down Expand Up @@ -2930,7 +2944,8 @@ void VPWidenStoreEVLRecipe::execute(VPTransformState &State) {
}
NewSI->addParamAttr(
1, Attribute::getWithAlignment(NewSI->getContext(), Alignment));
State.addMetadata(NewSI, cast<StoreInst>(&Ingredient));
State.addNewMetadata(NewSI, &Ingredient);
applyMetadata(*NewSI);
}

InstructionCost VPWidenStoreEVLRecipe::computeCost(ElementCount VF,
Expand Down
Loading