Skip to content

Commit

Permalink
[VPlan] Move DebugLoc to VPRecipeBase (NFCI).
Browse files Browse the repository at this point in the history
Add a dedicated debug location to VPRecipeBase to remove another
unneeded use of the underlying LLVM IR instruction and also consolidate
various DL fields in sub classes.

Each recipe can have debug location and it shouldn't rely on reference
to the underlying LLVM IR instructions to retain it. See various recipes
that had separate DL fields already.
  • Loading branch information
fhahn committed Sep 5, 2023
1 parent 0f1507a commit 165e24a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 52 deletions.
65 changes: 34 additions & 31 deletions llvm/lib/Transforms/Vectorize/VPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -709,13 +709,18 @@ class VPRecipeBase : public ilist_node_with_parent<VPRecipeBase, VPBasicBlock>,
/// Each VPRecipe belongs to a single VPBasicBlock.
VPBasicBlock *Parent = nullptr;

/// The debug location for the recipe.
DebugLoc DL;

public:
VPRecipeBase(const unsigned char SC, ArrayRef<VPValue *> Operands)
: VPDef(SC), VPUser(Operands, VPUser::VPUserID::Recipe) {}
VPRecipeBase(const unsigned char SC, ArrayRef<VPValue *> Operands,
DebugLoc DL = {})
: VPDef(SC), VPUser(Operands, VPUser::VPUserID::Recipe), DL(DL) {}

template <typename IterT>
VPRecipeBase(const unsigned char SC, iterator_range<IterT> Operands)
: VPDef(SC), VPUser(Operands, VPUser::VPUserID::Recipe) {}
VPRecipeBase(const unsigned char SC, iterator_range<IterT> Operands,
DebugLoc DL = {})
: VPDef(SC), VPUser(Operands, VPUser::VPUserID::Recipe), DL(DL) {}
virtual ~VPRecipeBase() = default;

/// \return the VPBasicBlock which this VPRecipe belongs to.
Expand Down Expand Up @@ -792,6 +797,9 @@ class VPRecipeBase : public ilist_node_with_parent<VPRecipeBase, VPBasicBlock>,
bool mayReadOrWriteMemory() const {
return mayReadFromMemory() || mayWriteToMemory();
}

/// Returns the debug location of the recipe.
DebugLoc getDebugLoc() const { return DL; }
};

// Helper macro to define common classof implementations for recipes.
Expand Down Expand Up @@ -862,15 +870,15 @@ class VPRecipeWithIRFlags : public VPRecipeBase {

public:
template <typename IterT>
VPRecipeWithIRFlags(const unsigned char SC, IterT Operands)
: VPRecipeBase(SC, Operands) {
VPRecipeWithIRFlags(const unsigned char SC, IterT Operands, DebugLoc DL = {})
: VPRecipeBase(SC, Operands, DL) {
OpType = OperationType::Other;
AllFlags = 0;
}

template <typename IterT>
VPRecipeWithIRFlags(const unsigned char SC, IterT Operands, Instruction &I)
: VPRecipeWithIRFlags(SC, Operands) {
: VPRecipeWithIRFlags(SC, Operands, I.getDebugLoc()) {
if (auto *Op = dyn_cast<CmpInst>(&I)) {
OpType = OperationType::Cmp;
CmpPredicate = Op->getPredicate();
Expand All @@ -891,20 +899,20 @@ class VPRecipeWithIRFlags : public VPRecipeBase {

template <typename IterT>
VPRecipeWithIRFlags(const unsigned char SC, IterT Operands,
CmpInst::Predicate Pred)
: VPRecipeBase(SC, Operands), OpType(OperationType::Cmp),
CmpInst::Predicate Pred, DebugLoc DL = {})
: VPRecipeBase(SC, Operands, DL), OpType(OperationType::Cmp),
CmpPredicate(Pred) {}

template <typename IterT>
VPRecipeWithIRFlags(const unsigned char SC, IterT Operands,
WrapFlagsTy WrapFlags)
: VPRecipeBase(SC, Operands), OpType(OperationType::OverflowingBinOp),
WrapFlagsTy WrapFlags, DebugLoc DL = {})
: VPRecipeBase(SC, Operands, DL), OpType(OperationType::OverflowingBinOp),
WrapFlags(WrapFlags) {}

template <typename IterT>
VPRecipeWithIRFlags(const unsigned char SC, IterT Operands,
FastMathFlags FMFs)
: VPRecipeBase(SC, Operands), OpType(OperationType::FPMathOp),
FastMathFlags FMFs, DebugLoc DL = {})
: VPRecipeBase(SC, Operands, DL), OpType(OperationType::FPMathOp),
FMFs(FMFs) {}

static inline bool classof(const VPRecipeBase *R) {
Expand Down Expand Up @@ -1030,7 +1038,6 @@ class VPInstruction : public VPRecipeWithIRFlags, public VPValue {
private:
typedef unsigned char OpcodeTy;
OpcodeTy Opcode;
DebugLoc DL;

/// An optional name that can be used for the generated IR instruction.
const std::string Name;
Expand All @@ -1053,8 +1060,8 @@ class VPInstruction : public VPRecipeWithIRFlags, public VPValue {
public:
VPInstruction(unsigned Opcode, ArrayRef<VPValue *> Operands, DebugLoc DL,
const Twine &Name = "")
: VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands), VPValue(this),
Opcode(Opcode), DL(DL), Name(Name.str()) {}
: VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands, DL),
VPValue(this), Opcode(Opcode), Name(Name.str()) {}

VPInstruction(unsigned Opcode, std::initializer_list<VPValue *> Operands,
DebugLoc DL = {}, const Twine &Name = "")
Expand All @@ -1065,8 +1072,8 @@ class VPInstruction : public VPRecipeWithIRFlags, public VPValue {

VPInstruction(unsigned Opcode, std::initializer_list<VPValue *> Operands,
WrapFlagsTy WrapFlags, DebugLoc DL = {}, const Twine &Name = "")
: VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands, WrapFlags),
VPValue(this), Opcode(Opcode), DL(DL), Name(Name.str()) {}
: VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands, WrapFlags, DL),
VPValue(this), Opcode(Opcode), Name(Name.str()) {}

VPInstruction(unsigned Opcode, std::initializer_list<VPValue *> Operands,
FastMathFlags FMFs, DebugLoc DL = {}, const Twine &Name = "");
Expand Down Expand Up @@ -1238,7 +1245,8 @@ class VPWidenCallRecipe : public VPRecipeBase, public VPValue {
struct VPWidenSelectRecipe : public VPRecipeBase, public VPValue {
template <typename IterT>
VPWidenSelectRecipe(SelectInst &I, iterator_range<IterT> Operands)
: VPRecipeBase(VPDef::VPWidenSelectSC, Operands), VPValue(this, &I) {}
: VPRecipeBase(VPDef::VPWidenSelectSC, Operands, I.getDebugLoc()),
VPValue(this, &I) {}

~VPWidenSelectRecipe() override = default;

Expand Down Expand Up @@ -1324,8 +1332,8 @@ class VPWidenGEPRecipe : public VPRecipeWithIRFlags, public VPValue {
class VPHeaderPHIRecipe : public VPRecipeBase, public VPValue {
protected:
VPHeaderPHIRecipe(unsigned char VPDefID, Instruction *UnderlyingInstr,
VPValue *Start = nullptr)
: VPRecipeBase(VPDefID, {}), VPValue(this, UnderlyingInstr) {
VPValue *Start = nullptr, DebugLoc DL = {})
: VPRecipeBase(VPDefID, {}, DL), VPValue(this, UnderlyingInstr) {
if (Start)
addOperand(Start);
}
Expand Down Expand Up @@ -1607,14 +1615,13 @@ class VPReductionPHIRecipe : public VPHeaderPHIRecipe {
/// A recipe for vectorizing a phi-node as a sequence of mask-based select
/// instructions.
class VPBlendRecipe : public VPRecipeBase, public VPValue {
PHINode *Phi;

public:
/// The blend operation is a User of the incoming values and of their
/// respective masks, ordered [I0, M0, I1, M1, ...]. Note that a single value
/// might be incoming with a full mask for which there is no VPValue.
VPBlendRecipe(PHINode *Phi, ArrayRef<VPValue *> Operands)
: VPRecipeBase(VPDef::VPBlendSC, Operands), VPValue(this, Phi), Phi(Phi) {
: VPRecipeBase(VPDef::VPBlendSC, Operands, Phi->getDebugLoc()),
VPValue(this, Phi) {
assert(Operands.size() > 0 &&
((Operands.size() == 1) || (Operands.size() % 2 == 0)) &&
"Expected either a single incoming value or a positive even number "
Expand Down Expand Up @@ -2047,11 +2054,9 @@ class VPExpandSCEVRecipe : public VPRecipeBase, public VPValue {
/// loop). VPWidenCanonicalIVRecipe represents the vector version of the
/// canonical induction variable.
class VPCanonicalIVPHIRecipe : public VPHeaderPHIRecipe {
DebugLoc DL;

public:
VPCanonicalIVPHIRecipe(VPValue *StartV, DebugLoc DL)
: VPHeaderPHIRecipe(VPDef::VPCanonicalIVPHISC, nullptr, StartV), DL(DL) {}
: VPHeaderPHIRecipe(VPDef::VPCanonicalIVPHISC, nullptr, StartV, DL) {}

~VPCanonicalIVPHIRecipe() override = default;

Expand Down Expand Up @@ -2094,12 +2099,10 @@ class VPCanonicalIVPHIRecipe : public VPHeaderPHIRecipe {
/// TODO: It would be good to use the existing VPWidenPHIRecipe instead and
/// remove VPActiveLaneMaskPHIRecipe.
class VPActiveLaneMaskPHIRecipe : public VPHeaderPHIRecipe {
DebugLoc DL;

public:
VPActiveLaneMaskPHIRecipe(VPValue *StartMask, DebugLoc DL)
: VPHeaderPHIRecipe(VPDef::VPActiveLaneMaskPHISC, nullptr, StartMask),
DL(DL) {}
: VPHeaderPHIRecipe(VPDef::VPActiveLaneMaskPHISC, nullptr, StartMask,
DL) {}

~VPActiveLaneMaskPHIRecipe() override = default;

Expand Down
35 changes: 14 additions & 21 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,25 +250,25 @@ VPInstruction::VPInstruction(unsigned Opcode, CmpInst::Predicate Pred,
VPValue *A, VPValue *B, DebugLoc DL,
const Twine &Name)
: VPRecipeWithIRFlags(VPDef::VPInstructionSC, ArrayRef<VPValue *>({A, B}),
Pred),
VPValue(this), Opcode(Opcode), DL(DL), Name(Name.str()) {
Pred, DL),
VPValue(this), Opcode(Opcode), Name(Name.str()) {
assert(Opcode == Instruction::ICmp &&
"only ICmp predicates supported at the moment");
}

VPInstruction::VPInstruction(unsigned Opcode,
std::initializer_list<VPValue *> Operands,
FastMathFlags FMFs, DebugLoc DL, const Twine &Name)
: VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands, FMFs),
VPValue(this), Opcode(Opcode), DL(DL), Name(Name.str()) {
: VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands, FMFs, DL),
VPValue(this), Opcode(Opcode), Name(Name.str()) {
// Make sure the VPInstruction is a floating-point operation.
assert(isFPMathOp() && "this op can't take fast-math flags");
}

Value *VPInstruction::generateInstruction(VPTransformState &State,
unsigned Part) {
IRBuilderBase &Builder = State.Builder;
Builder.SetCurrentDebugLocation(DL);
Builder.SetCurrentDebugLocation(getDebugLoc());

if (Instruction::isBinaryOp(getOpcode())) {
Value *A = State.get(getOperand(0), Part);
Expand Down Expand Up @@ -488,7 +488,7 @@ void VPInstruction::print(raw_ostream &O, const Twine &Indent,
printFlags(O);
printOperands(O, SlotTracker);

if (DL) {
if (auto DL = getDebugLoc()) {
O << ", !dbg ";
DL.print(O);
}
Expand Down Expand Up @@ -591,8 +591,7 @@ void VPWidenSelectRecipe::print(raw_ostream &O, const Twine &Indent,
#endif

void VPWidenSelectRecipe::execute(VPTransformState &State) {
auto &I = *cast<SelectInst>(getUnderlyingInstr());
State.setDebugLocFrom(I.getDebugLoc());
State.setDebugLocFrom(getDebugLoc());

// The condition can be loop invariant but still defined inside the
// loop. This means that we can't just use the original 'cond' value.
Expand All @@ -607,7 +606,7 @@ void VPWidenSelectRecipe::execute(VPTransformState &State) {
Value *Op1 = State.get(getOperand(2), Part);
Value *Sel = State.Builder.CreateSelect(Cond, Op0, Op1);
State.set(this, Sel, Part);
State.addMetadata(Sel, &I);
State.addMetadata(Sel, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
}
}

Expand Down Expand Up @@ -654,6 +653,7 @@ void VPRecipeWithIRFlags::printFlags(raw_ostream &O) const {
#endif

void VPWidenRecipe::execute(VPTransformState &State) {
State.setDebugLocFrom(getDebugLoc());
auto &I = *cast<Instruction>(getUnderlyingValue());
auto &Builder = State.Builder;
switch (I.getOpcode()) {
Expand Down Expand Up @@ -683,8 +683,6 @@ void VPWidenRecipe::execute(VPTransformState &State) {
case Instruction::Or:
case Instruction::Xor: {
// Just widen unops and binops.
State.setDebugLocFrom(I.getDebugLoc());

for (unsigned Part = 0; Part < State.UF; ++Part) {
SmallVector<Value *, 2> Ops;
for (VPValue *VPOp : operands())
Expand All @@ -703,8 +701,6 @@ void VPWidenRecipe::execute(VPTransformState &State) {
break;
}
case Instruction::Freeze: {
State.setDebugLocFrom(I.getDebugLoc());

for (unsigned Part = 0; Part < State.UF; ++Part) {
Value *Op = State.get(getOperand(0), Part);

Expand All @@ -718,7 +714,6 @@ void VPWidenRecipe::execute(VPTransformState &State) {
// Widen compares. Generate vector compares.
bool FCmp = (I.getOpcode() == Instruction::FCmp);
auto *Cmp = cast<CmpInst>(&I);
State.setDebugLocFrom(Cmp->getDebugLoc());
for (unsigned Part = 0; Part < State.UF; ++Part) {
Value *A = State.get(getOperand(0), Part);
Value *B = State.get(getOperand(1), Part);
Expand Down Expand Up @@ -756,9 +751,7 @@ void VPWidenRecipe::print(raw_ostream &O, const Twine &Indent,
#endif

void VPWidenCastRecipe::execute(VPTransformState &State) {
auto *I = cast_or_null<Instruction>(getUnderlyingValue());
if (I)
State.setDebugLocFrom(I->getDebugLoc());
State.setDebugLocFrom(getDebugLoc());
auto &Builder = State.Builder;
/// Vectorize casts.
assert(State.VF.isVector() && "Not vectorizing?");
Expand All @@ -768,7 +761,7 @@ void VPWidenCastRecipe::execute(VPTransformState &State) {
Value *A = State.get(getOperand(0), Part);
Value *Cast = Builder.CreateCast(Instruction::CastOps(Opcode), A, DestTy);
State.set(this, Cast, Part);
State.addMetadata(Cast, I);
State.addMetadata(Cast, cast_or_null<Instruction>(getUnderlyingValue()));
}
}

Expand Down Expand Up @@ -1193,7 +1186,7 @@ void VPWidenGEPRecipe::print(raw_ostream &O, const Twine &Indent,
#endif

void VPBlendRecipe::execute(VPTransformState &State) {
State.setDebugLocFrom(Phi->getDebugLoc());
State.setDebugLocFrom(getDebugLoc());
// We know that all PHIs in non-header blocks are converted into
// selects, so we don't have to worry about the insertion order and we
// can just use the builder.
Expand Down Expand Up @@ -1417,7 +1410,7 @@ void VPCanonicalIVPHIRecipe::execute(VPTransformState &State) {

BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
EntryPart->addIncoming(Start, VectorPH);
EntryPart->setDebugLoc(DL);
EntryPart->setDebugLoc(getDebugLoc());
for (unsigned Part = 0, UF = State.UF; Part < UF; ++Part)
State.set(this, EntryPart, Part);
}
Expand Down Expand Up @@ -1687,7 +1680,7 @@ void VPActiveLaneMaskPHIRecipe::execute(VPTransformState &State) {
PHINode *EntryPart =
State.Builder.CreatePHI(StartMask->getType(), 2, "active.lane.mask");
EntryPart->addIncoming(StartMask, VectorPH);
EntryPart->setDebugLoc(DL);
EntryPart->setDebugLoc(getDebugLoc());
State.set(this, EntryPart, Part);
}
}
Expand Down

0 comments on commit 165e24a

Please sign in to comment.