[llvm] 8a16422 - [RemoveDIs] Add DPLabels support [3a/3] (#82633)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 23 03:37:25 PST 2024
Author: Orlando Cazalet-Hyams
Date: 2024-02-23T11:37:21Z
New Revision: 8a164220207b579c31d6aa6552944441c83e9465
URL: https://github.com/llvm/llvm-project/commit/8a164220207b579c31d6aa6552944441c83e9465
DIFF: https://github.com/llvm/llvm-project/commit/8a164220207b579c31d6aa6552944441c83e9465.diff
LOG: [RemoveDIs] Add DPLabels support [3a/3] (#82633)
Patch 2 of 3 to add llvm.dbg.label support to the RemoveDIs project. The
patch stack adds the DPLabel class, which is the RemoveDIs llvm.dbg.label
equivalent.
1. Add DbgRecord base class for DPValue and the not-yet-added
DPLabel class.
2. Add the DPLabel class.
-> 3. Add support to passes.
The next patch, #82639, will enable conversion between dbg.labels and DPLabels.
AssignemntTrackingAnalysis support could have gone two ways:
1. Have the analysis store a DPLabel representation in its results -
SelectionDAGBuilder reads the analysis results and ignores all DbgRecord
kinds.
2. Ignore DPLabels in the analysis - SelectionDAGBuilder reads the analysis
results but still needs to iterate over DPLabels from the IR.
I went with option 2 because it's less work and is no less correct than 1. It's
worth noting that causes labels to sink to the bottom of packs of debug records.
e.g., [value, label, value] becomes [value, value, label]. This shouldn't be a
problem because labels and variable locations don't have an ordering requirement.
The ordering between variable locations is maintained and the label movement is
deterministic
Added:
Modified:
llvm/include/llvm/IR/DebugProgramInstruction.h
llvm/include/llvm/IR/IntrinsicInst.h
llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
llvm/lib/Transforms/Utils/ValueMapper.cpp
llvm/test/Transforms/SpeculativeExecution/PR46267.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index 1c8619741eb69f..84b0f743d3c9b9 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -157,6 +157,11 @@ class DbgRecord : public ilist_node<DbgRecord> {
~DbgRecord() = default;
};
+inline raw_ostream &operator<<(raw_ostream &OS, const DbgRecord &R) {
+ R.print(OS);
+ return OS;
+}
+
/// Records a position in IR for a source label (DILabel). Corresponds to the
/// llvm.dbg.label intrinsic.
/// FIXME: Rename DbgLabelRecord when DPValue is renamed to DbgVariableRecord.
@@ -536,11 +541,6 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DPMarker &Marker) {
return OS;
}
-inline raw_ostream &operator<<(raw_ostream &OS, const DPValue &Value) {
- Value.print(OS);
- return OS;
-}
-
/// Inline helper to return a range of DPValues attached to a marker. It needs
/// to be inlined as it's frequently called, but also come after the declaration
/// of DPMarker. Thus: it's pre-declared by users like Instruction, then an
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index b8d578d0fee082..fbaaef8ea44315 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -531,6 +531,9 @@ class DbgAssignIntrinsic : public DbgValueInst {
class DbgLabelInst : public DbgInfoIntrinsic {
public:
DILabel *getLabel() const { return cast<DILabel>(getRawLabel()); }
+ void setLabel(DILabel *NewLabel) {
+ setArgOperand(0, MetadataAsValue::get(getContext(), NewLabel));
+ }
Metadata *getRawLabel() const {
return cast<MetadataAsValue>(getArgOperand(0))->getMetadata();
diff --git a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
index 7b66a851db2527..3b84624c3d4dca 100644
--- a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
+++ b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
@@ -829,11 +829,7 @@ class MemLocFragmentFill {
void process(BasicBlock &BB, VarFragMap &LiveSet) {
BBInsertBeforeMap[&BB].clear();
for (auto &I : BB) {
- for (DbgRecord &DR : I.getDbgValueRange()) {
- // FIXME: DPValue::filter usage needs attention in this file; we need
- // to make sure dbg.labels are handled correctly in RemoveDIs mode.
- // Cast below to ensure this gets fixed when DPLabels are introduced.
- DPValue &DPV = cast<DPValue>(DR);
+ for (DPValue &DPV : DPValue::filter(I.getDbgValueRange())) {
if (const auto *Locs = FnVarLocs->getWedge(&DPV)) {
for (const VarLocInfo &Loc : *Locs) {
addDef(Loc, &DPV, *I.getParent(), LiveSet);
@@ -1919,6 +1915,9 @@ void AssignmentTrackingLowering::process(BasicBlock &BB, BlockInfo *LiveSet) {
// attached DPValues, or a non-debug instruction with attached unprocessed
// DPValues.
if (II != EI && II->hasDbgValues()) {
+ // Skip over non-variable debug records (i.e., labels). They're going to
+ // be read from IR (possibly re-ordering them within the debug record
+ // range) rather than from the analysis results.
for (DPValue &DPV : DPValue::filter(II->getDbgValueRange())) {
resetInsertionPoint(DPV);
processDPValue(DPV, LiveSet);
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 7c95cef2eeb761..38bb808dd5bd53 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -3275,7 +3275,17 @@ void IRTranslator::translateDbgDeclareRecord(Value *Address, bool HasArgList,
void IRTranslator::translateDbgInfo(const Instruction &Inst,
MachineIRBuilder &MIRBuilder) {
- for (DPValue &DPV : DPValue::filter(Inst.getDbgValueRange())) {
+ for (DbgRecord &DR : Inst.getDbgValueRange()) {
+ if (DPLabel *DPL = dyn_cast<DPLabel>(&DR)) {
+ MIRBuilder.setDebugLoc(DPL->getDebugLoc());
+ assert(DPL->getLabel() && "Missing label");
+ assert(DPL->getLabel()->isValidLocationForIntrinsic(
+ MIRBuilder.getDebugLoc()) &&
+ "Expected inlined-at fields to agree");
+ MIRBuilder.buildDbgLabel(DPL->getLabel());
+ continue;
+ }
+ DPValue &DPV = cast<DPValue>(DR);
const DILocalVariable *Variable = DPV.getVariable();
const DIExpression *Expression = DPV.getExpression();
Value *V = DPV.getVariableLocationOp(0);
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index 5651498dd3f5aa..246762dd7ab628 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1188,11 +1188,24 @@ void FastISel::handleDbgInfo(const Instruction *II) {
MIMD = MIMetadata();
// Reverse order of debug records, because fast-isel walks through backwards.
- for (DbgRecord &DPR : llvm::reverse(II->getDbgValueRange())) {
+ for (DbgRecord &DR : llvm::reverse(II->getDbgValueRange())) {
flushLocalValueMap();
recomputeInsertPt();
- DPValue &DPV = cast<DPValue>(DPR);
+ if (DPLabel *DPL = dyn_cast<DPLabel>(&DR)) {
+ assert(DPL->getLabel() && "Missing label");
+ if (!FuncInfo.MF->getMMI().hasDebugInfo()) {
+ LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DPL << "\n");
+ continue;
+ }
+
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DPL->getDebugLoc(),
+ TII.get(TargetOpcode::DBG_LABEL))
+ .addMetadata(DPL->getLabel());
+ continue;
+ }
+
+ DPValue &DPV = cast<DPValue>(DR);
Value *V = nullptr;
if (!DPV.hasArgList())
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index e893a5b616d33e..ee600d389c2cc3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1241,17 +1241,30 @@ void SelectionDAGBuilder::visitDbgInfo(const Instruction &I) {
It->Expr, Vals.size() > 1, It->DL, SDNodeOrder);
}
}
- // We must early-exit here to prevent any DPValues from being emitted below,
- // as we have just emitted the debug values resulting from assignment
- // tracking analysis, making any existing DPValues redundant (and probably
- // less correct).
- return;
}
+ // We must skip DPValues if they've already been processed above as we
+ // have just emitted the debug values resulting from assignment tracking
+ // analysis, making any existing DPValues redundant (and probably less
+ // correct). We still need to process DPLabels. This does sink DPLabels
+ // to the bottom of the group of debug records. That sholdn't be important
+ // as it does so deterministcally and ordering between DPLabels and DPValues
+ // is immaterial (other than for MIR/IR printing).
+ bool SkipDPValues = DAG.getFunctionVarLocs();
// Is there is any debug-info attached to this instruction, in the form of
- // DPValue non-instruction debug-info records.
- for (DbgRecord &DPR : I.getDbgValueRange()) {
- DPValue &DPV = cast<DPValue>(DPR);
+ // DbgRecord non-instruction debug-info records.
+ for (DbgRecord &DR : I.getDbgValueRange()) {
+ if (DPLabel *DPL = dyn_cast<DPLabel>(&DR)) {
+ assert(DPL->getLabel() && "Missing label");
+ SDDbgLabel *SDV =
+ DAG.getDbgLabel(DPL->getLabel(), DPL->getDebugLoc(), SDNodeOrder);
+ DAG.AddDbgLabel(SDV);
+ continue;
+ }
+
+ if (SkipDPValues)
+ continue;
+ DPValue &DPV = cast<DPValue>(DR);
DILocalVariable *Variable = DPV.getVariable();
DIExpression *Expression = DPV.getExpression();
dropDanglingDebugInfo(Variable, Expression);
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index c2a470c5fc7162..fba404c9b027cb 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -1141,12 +1141,14 @@ void SlotTracker::processFunctionMetadata(const Function &F) {
void SlotTracker::processDbgRecordMetadata(const DbgRecord &DR) {
if (const DPValue *DPV = dyn_cast<const DPValue>(&DR)) {
CreateMetadataSlot(DPV->getVariable());
- CreateMetadataSlot(DPV->getDebugLoc());
if (DPV->isDbgAssign())
CreateMetadataSlot(DPV->getAssignID());
+ } else if (const DPLabel *DPL = dyn_cast<const DPLabel>(&DR)) {
+ CreateMetadataSlot(DPL->getLabel());
} else {
llvm_unreachable("unsupported DbgRecord kind");
}
+ CreateMetadataSlot(DR.getDebugLoc());
}
void SlotTracker::processInstructionMetadata(const Instruction &I) {
diff --git a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
index f4f3070d11c7bb..260f31b59ed29a 100644
--- a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
+++ b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
@@ -291,9 +291,9 @@ bool SpeculativeExecutionPass::considerHoistingFromTo(
InstructionCost TotalSpeculationCost = 0;
unsigned NotHoistedInstCount = 0;
for (const auto &I : FromBlock) {
- // Make note of any DPValues that need hoisting.
- for (DbgRecord &DR : I.getDbgValueRange()) {
- DPValue &DPV = cast<DPValue>(DR);
+ // Make note of any DPValues that need hoisting. DPLabels
+ // get left behind just like llvm.dbg.labels.
+ for (DPValue &DPV : DPValue::filter(I.getDbgValueRange())) {
if (HasNoUnhoistedInstr(DPV.location_ops()))
DPValuesToHoist[DPV.getInstruction()].push_back(&DPV);
}
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 7fd6759a61fbae..5bb109a04ff178 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -386,7 +386,15 @@ static bool DPValuesRemoveRedundantDbgInstrsUsingBackwardScan(BasicBlock *BB) {
SmallVector<DPValue *, 8> ToBeRemoved;
SmallDenseSet<DebugVariable> VariableSet;
for (auto &I : reverse(*BB)) {
- for (DPValue &DPV : reverse(DPValue::filter(I.getDbgValueRange()))) {
+ for (DbgRecord &DR : reverse(I.getDbgValueRange())) {
+ if (isa<DPLabel>(DR)) {
+ // Emulate existing behaviour (see comment below for dbg.declares).
+ // FIXME: Don't do this.
+ VariableSet.clear();
+ continue;
+ }
+
+ DPValue &DPV = cast<DPValue>(DR);
// Skip declare-type records, as the debug intrinsic method only works
// on dbg.value intrinsics.
if (DPV.getType() == DPValue::LocationType::Declare) {
diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
index 8ebcf0c04fd5a9..bab065153f3efa 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1585,8 +1585,30 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
return cast<DILocalVariable>(NewVar);
};
- auto UpdateDPValuesOnInst = [&](Instruction &I) -> void {
- for (DPValue &DPV : DPValue::filter(I.getDbgValueRange())) {
+ auto UpdateDbgLabel = [&](auto *LabelRecord) {
+ // Point the label record to a fresh label within the new function if
+ // the record was not inlined from some other function.
+ if (LabelRecord->getDebugLoc().getInlinedAt())
+ return;
+ DILabel *OldLabel = LabelRecord->getLabel();
+ DINode *&NewLabel = RemappedMetadata[OldLabel];
+ if (!NewLabel) {
+ DILocalScope *NewScope = DILocalScope::cloneScopeForSubprogram(
+ *OldLabel->getScope(), *NewSP, Ctx, Cache);
+ NewLabel = DILabel::get(Ctx, NewScope, OldLabel->getName(),
+ OldLabel->getFile(), OldLabel->getLine());
+ }
+ LabelRecord->setLabel(cast<DILabel>(NewLabel));
+ };
+
+ auto UpdateDbgRecordsOnInst = [&](Instruction &I) -> void {
+ for (DbgRecord &DR : I.getDbgValueRange()) {
+ if (DPLabel *DPL = dyn_cast<DPLabel>(&DR)) {
+ UpdateDbgLabel(DPL);
+ continue;
+ }
+
+ DPValue &DPV = cast<DPValue>(DR);
// Apply the two updates that dbg.values get: invalid operands, and
// variable metadata fixup.
if (any_of(DPV.location_ops(), IsInvalidLocation)) {
@@ -1599,13 +1621,11 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
}
if (!DPV.getDebugLoc().getInlinedAt())
DPV.setVariable(GetUpdatedDIVariable(DPV.getVariable()));
- DPV.setDebugLoc(DebugLoc::replaceInlinedAtSubprogram(DPV.getDebugLoc(),
- *NewSP, Ctx, Cache));
}
};
for (Instruction &I : instructions(NewFunc)) {
- UpdateDPValuesOnInst(I);
+ UpdateDbgRecordsOnInst(I);
auto *DII = dyn_cast<DbgInfoIntrinsic>(&I);
if (!DII)
@@ -1614,17 +1634,7 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
// Point the intrinsic to a fresh label within the new function if the
// intrinsic was not inlined from some other function.
if (auto *DLI = dyn_cast<DbgLabelInst>(&I)) {
- if (DLI->getDebugLoc().getInlinedAt())
- continue;
- DILabel *OldLabel = DLI->getLabel();
- DINode *&NewLabel = RemappedMetadata[OldLabel];
- if (!NewLabel) {
- DILocalScope *NewScope = DILocalScope::cloneScopeForSubprogram(
- *OldLabel->getScope(), *NewSP, Ctx, Cache);
- NewLabel = DILabel::get(Ctx, NewScope, OldLabel->getName(),
- OldLabel->getFile(), OldLabel->getLine());
- }
- DLI->setArgOperand(0, MetadataAsValue::get(Ctx, NewLabel));
+ UpdateDbgLabel(DLI);
continue;
}
@@ -1658,6 +1668,9 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
if (const DebugLoc &DL = I.getDebugLoc())
I.setDebugLoc(
DebugLoc::replaceInlinedAtSubprogram(DL, *NewSP, Ctx, Cache));
+ for (DbgRecord &DR : I.getDbgValueRange())
+ DR.setDebugLoc(DebugLoc::replaceInlinedAtSubprogram(DR.getDebugLoc(),
+ *NewSP, Ctx, Cache));
// Loop info metadata may contain line locations. Fix them up.
auto updateLoopInfoLoc = [&Ctx, &Cache, NewSP](Metadata *MD) -> Metadata * {
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index 08fdd3b75ffcbd..2ff7c015107677 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -111,8 +111,7 @@ Instruction *getUntagLocationIfFunctionExit(Instruction &Inst) {
void StackInfoBuilder::visit(Instruction &Inst) {
// Visit non-intrinsic debug-info records attached to Inst.
- for (DbgRecord &DR : Inst.getDbgValueRange()) {
- DPValue &DPV = cast<DPValue>(DR);
+ for (DPValue &DPV : DPValue::filter(Inst.getDbgValueRange())) {
auto AddIfInteresting = [&](Value *V) {
if (auto *AI = dyn_cast_or_null<AllocaInst>(V)) {
if (!isInterestingAlloca(*AI))
diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp
index 6e46469f5a601e..91ab2795a4b9d3 100644
--- a/llvm/lib/Transforms/Utils/ValueMapper.cpp
+++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp
@@ -538,6 +538,11 @@ Value *Mapper::mapValue(const Value *V) {
}
void Mapper::remapDPValue(DbgRecord &DR) {
+ if (DPLabel *DPL = dyn_cast<DPLabel>(&DR)) {
+ DPL->setLabel(cast<DILabel>(mapMetadata(DPL->getLabel())));
+ return;
+ }
+
DPValue &V = cast<DPValue>(DR);
// Remap variables and DILocations.
auto *MappedVar = mapMetadata(V.getVariable());
diff --git a/llvm/test/Transforms/SpeculativeExecution/PR46267.ll b/llvm/test/Transforms/SpeculativeExecution/PR46267.ll
index c27b492b4b8765..d940ee6a7863d7 100644
--- a/llvm/test/Transforms/SpeculativeExecution/PR46267.ll
+++ b/llvm/test/Transforms/SpeculativeExecution/PR46267.ll
@@ -41,12 +41,16 @@ land.rhs: ; preds = %entry
; CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %y
; CHECK-NEXT: %a0 = load i32, ptr undef, align 1
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %a0
+; CHECK-NEXT: call void @llvm.dbg.label
call void @llvm.dbg.label(metadata !11), !dbg !10
%y = alloca i32, align 4
call void @llvm.dbg.declare(metadata ptr %y, metadata !14, metadata !DIExpression()), !dbg !10
%a0 = load i32, ptr undef, align 1
call void @llvm.dbg.value(metadata i32 %a0, metadata !9, metadata !DIExpression()), !dbg !10
+ ;; RemoveDIs: Check a label that is attached to a hoisted instruction
+ ;; gets left behind (match intrinsic-style debug info behaviour).
+ call void @llvm.dbg.label(metadata !15), !dbg !10
%a2 = add i32 %i, 0
call void @llvm.dbg.value(metadata i32 %a2, metadata !13, metadata !DIExpression()), !dbg !10
@@ -82,3 +86,4 @@ attributes #1 = { nounwind readnone speculatable willreturn }
!12 = !DILocalVariable(name: "x", scope: !6, file: !1, line: 3, type: !4)
!13 = !DILocalVariable(name: "a2", scope: !6, file: !1, line: 3, type: !4)
!14 = !DILocalVariable(name: "y", scope: !6, file: !1, line: 3, type: !4)
+!15 = !DILabel(scope: !6, name: "label2", file: !1, line: 2)
More information about the llvm-commits
mailing list