[RemoveDIs] Handle DPValues in FastISel #76952
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
backend:ARM
backend:AArch64
backend:X86
debuginfo
llvm:SelectionDAG
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-backend-aarch64 Author: Orlando Cazalet-Hyams (OCHyams) ChangesThe change is fairly mechanical:
The test updates look the same as other RemoveDIs changes: re-run the tests with Patch is 70.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76952.diff 74 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/FastISel.h b/llvm/include/llvm/CodeGen/FastISel.h
index dc2931b40d352c..35d9aeda378287 100644
--- a/llvm/include/llvm/CodeGen/FastISel.h
+++ b/llvm/include/llvm/CodeGen/FastISel.h
@@ -319,6 +319,8 @@ class FastISel {
/// Reset InsertPt to the given old insert position.
void leaveLocalValueArea(SavePoint Old);
+ void handleDbgInfo(const Instruction *II);
+
protected:
explicit FastISel(FunctionLoweringInfo &FuncInfo,
const TargetLibraryInfo *LibInfo,
@@ -342,6 +344,18 @@ class FastISel {
/// specific intrinsic lowering. It returns true if it was successful.
virtual bool fastLowerIntrinsicCall(const IntrinsicInst *II);
+ /// This method is called by target-independent code to do target-specific
+ /// lowering of debug information. It returns false if the debug information
+ /// couldn't be lowered and was instead discarded.
+ virtual bool lowerDbgValue(const Value *V, DIExpression *Expr,
+ DILocalVariable *Var, const DebugLoc &DL);
+
+ /// This method is called by target-independent code to do target-specific
+ /// lowering of debug information. It returns false if the debug information
+ /// couldn't be lowered and was instead discarded.
+ virtual bool lowerDbgDeclare(const Value *V, DIExpression *Expr,
+ DILocalVariable *Var, const DebugLoc &DL);
+
/// This method is called by target-independent code to request that an
/// instruction with the given type and opcode be emitted.
virtual unsigned fastEmit_(MVT VT, MVT RetVT, unsigned Opcode);
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index f3d8edb8926b66..abba685f151a08 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1182,6 +1182,184 @@ bool FastISel::selectCall(const User *I) {
return lowerCall(Call);
}
+void FastISel::handleDbgInfo(const Instruction *II) {
+ if (!II->hasDbgValues())
+ return;
+
+ // Clear any metadata.
+ MIMD = MIMetadata();
+
+ // Reverse order of debug records, because fast-isel walks through backwards.
+ for (DPValue &DPV : llvm::reverse(II->getDbgValueRange())) {
+ flushLocalValueMap();
+ recomputeInsertPt();
+
+ Value *V = nullptr;
+ if (!DPV.hasArgList())
+ V = DPV.getVariableLocationOp(0);
+
+ bool Res = false;
+ if (DPV.getType() == DPValue::LocationType::Value) {
+ Res = lowerDbgValue(V, DPV.getExpression(), DPV.getVariable(),
+ DPV.getDebugLoc());
+ } else {
+ assert(DPV.getType() == DPValue::LocationType::Declare);
+ if (FuncInfo.PreprocessedDPVDeclares.contains(&DPV))
+ continue;
+ Res = lowerDbgDeclare(V, DPV.getExpression(), DPV.getVariable(),
+ DPV.getDebugLoc());
+ }
+
+ if (!Res)
+ LLVM_DEBUG(dbgs() << "Dropping debug-info for " << DPV << "\n";);
+ }
+}
+
+bool FastISel::lowerDbgValue(const Value *V, DIExpression *Expr,
+ DILocalVariable *Var, const DebugLoc &DL) {
+ // This form of DBG_VALUE is target-independent.
+ const MCInstrDesc &II = TII.get(TargetOpcode::DBG_VALUE);
+ if (!V || isa<UndefValue>(V)) {
+ // DI is either undef or cannot produce a valid DBG_VALUE, so produce an
+ // undef DBG_VALUE to terminate any prior location.
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, II, false, 0U, Var, Expr);
+ return true;
+ }
+ if (const auto *CI = dyn_cast<ConstantInt>(V)) {
+ // See if there's an expression to constant-fold.
+ if (Expr)
+ std::tie(Expr, CI) = Expr->constantFold(CI);
+ if (CI->getBitWidth() > 64)
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, II)
+ .addCImm(CI)
+ .addImm(0U)
+ .addMetadata(Var)
+ .addMetadata(Expr);
+ else
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, II)
+ .addImm(CI->getZExtValue())
+ .addImm(0U)
+ .addMetadata(Var)
+ .addMetadata(Expr);
+ return true;
+ }
+ if (const auto *CF = dyn_cast<ConstantFP>(V)) {
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, II)
+ .addFPImm(CF)
+ .addImm(0U)
+ .addMetadata(Var)
+ .addMetadata(Expr);
+ return true;
+ }
+ if (const auto *Arg = dyn_cast<Argument>(V);
+ Arg && Expr && Expr->isEntryValue()) {
+ // As per the Verifier, this case is only valid for swift async Args.
+ assert(Arg->hasAttribute(Attribute::AttrKind::SwiftAsync));
+
+ Register Reg = getRegForValue(Arg);
+ for (auto [PhysReg, VirtReg] : FuncInfo.RegInfo->liveins())
+ if (Reg == VirtReg || Reg == PhysReg) {
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, II, false /*IsIndirect*/,
+ PhysReg, Var, Expr);
+ return true;
+ }
+
+ LLVM_DEBUG(dbgs() << "Dropping dbg.value: expression is entry_value but "
+ "couldn't find a physical register\n");
+ return false;
+ }
+ if (auto SI = FuncInfo.StaticAllocaMap.find(dyn_cast<AllocaInst>(V));
+ SI != FuncInfo.StaticAllocaMap.end()) {
+ MachineOperand FrameIndexOp = MachineOperand::CreateFI(SI->second);
+ bool IsIndirect = false;
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, II, IsIndirect, FrameIndexOp,
+ Var, Expr);
+ return true;
+ }
+ if (Register Reg = lookUpRegForValue(V)) {
+ // FIXME: This does not handle register-indirect values at offset 0.
+ if (!FuncInfo.MF->useDebugInstrRef()) {
+ bool IsIndirect = false;
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL, II, IsIndirect, Reg, Var,
+ Expr);
+ return true;
+ }
+ // If using instruction referencing, produce this as a DBG_INSTR_REF,
+ // to be later patched up by finalizeDebugInstrRefs.
+ SmallVector<MachineOperand, 1> MOs({MachineOperand::CreateReg(
+ /* Reg */ Reg, /* isDef */ false, /* isImp */ false,
+ /* isKill */ false, /* isDead */ false,
+ /* isUndef */ false, /* isEarlyClobber */ false,
+ /* SubReg */ 0, /* isDebug */ true)});
+ SmallVector<uint64_t, 2> Ops({dwarf::DW_OP_LLVM_arg, 0});
+ auto *NewExpr = DIExpression::prependOpcodes(Expr, Ops);
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL,
+ TII.get(TargetOpcode::DBG_INSTR_REF), /*IsIndirect*/ false, MOs,
+ Var, NewExpr);
+ return true;
+ }
+ return false;
+}
+
+bool FastISel::lowerDbgDeclare(const Value *Address, DIExpression *Expr,
+ DILocalVariable *Var, const DebugLoc &DL) {
+ if (!Address || isa<UndefValue>(Address)) {
+ LLVM_DEBUG(dbgs() << "Dropping debug info (bad/undef address)\n");
+ return false;
+ }
+
+ std::optional<MachineOperand> Op;
+ if (Register Reg = lookUpRegForValue(Address))
+ Op = MachineOperand::CreateReg(Reg, false);
+
+ // If we have a VLA that has a "use" in a metadata node that's then used
+ // here but it has no other uses, then we have a problem. E.g.,
+ //
+ // int foo (const int *x) {
+ // char a[*x];
+ // return 0;
+ // }
+ //
+ // If we assign 'a' a vreg and fast isel later on has to use the selection
+ // DAG isel, it will want to copy the value to the vreg. However, there are
+ // no uses, which goes counter to what selection DAG isel expects.
+ if (!Op && !Address->use_empty() && isa<Instruction>(Address) &&
+ (!isa<AllocaInst>(Address) ||
+ !FuncInfo.StaticAllocaMap.count(cast<AllocaInst>(Address))))
+ Op = MachineOperand::CreateReg(FuncInfo.InitializeRegForValue(Address),
+ false);
+
+ if (Op) {
+ assert(Var->isValidLocationForIntrinsic(DL) &&
+ "Expected inlined-at fields to agree");
+ if (FuncInfo.MF->useDebugInstrRef() && Op->isReg()) {
+ // If using instruction referencing, produce this as a DBG_INSTR_REF,
+ // to be later patched up by finalizeDebugInstrRefs. Tack a deref onto
+ // the expression, we don't have an "indirect" flag in DBG_INSTR_REF.
+ SmallVector<uint64_t, 3> Ops(
+ {dwarf::DW_OP_LLVM_arg, 0, dwarf::DW_OP_deref});
+ auto *NewExpr = DIExpression::prependOpcodes(Expr, Ops);
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL,
+ TII.get(TargetOpcode::DBG_INSTR_REF), /*IsIndirect*/ false, *Op,
+ Var, NewExpr);
+ return true;
+ }
+
+ // A dbg.declare describes the address of a source variable, so lower it
+ // into an indirect DBG_VALUE.
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL,
+ TII.get(TargetOpcode::DBG_VALUE), /*IsIndirect*/ true, *Op, Var,
+ Expr);
+ return true;
+ }
+
+ // We can't yet handle anything else here because it would require
+ // generating code, thus altering codegen because of debug info.
+ LLVM_DEBUG(
+ dbgs() << "Dropping debug info (no materialized reg for address)\n");
+ return false;
+}
+
bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) {
switch (II->getIntrinsicID()) {
default:
@@ -1211,153 +1389,28 @@ bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) {
return true;
const Value *Address = DI->getAddress();
- if (!Address || isa<UndefValue>(Address)) {
- LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI
- << " (bad/undef address)\n");
- return true;
- }
+ if (!lowerDbgDeclare(Address, DI->getExpression(), DI->getVariable(),
+ MIMD.getDL()))
+ LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI);
- std::optional<MachineOperand> Op;
- if (Register Reg = lookUpRegForValue(Address))
- Op = MachineOperand::CreateReg(Reg, false);
-
- // If we have a VLA that has a "use" in a metadata node that's then used
- // here but it has no other uses, then we have a problem. E.g.,
- //
- // int foo (const int *x) {
- // char a[*x];
- // return 0;
- // }
- //
- // If we assign 'a' a vreg and fast isel later on has to use the selection
- // DAG isel, it will want to copy the value to the vreg. However, there are
- // no uses, which goes counter to what selection DAG isel expects.
- if (!Op && !Address->use_empty() && isa<Instruction>(Address) &&
- (!isa<AllocaInst>(Address) ||
- !FuncInfo.StaticAllocaMap.count(cast<AllocaInst>(Address))))
- Op = MachineOperand::CreateReg(FuncInfo.InitializeRegForValue(Address),
- false);
-
- if (Op) {
- assert(DI->getVariable()->isValidLocationForIntrinsic(MIMD.getDL()) &&
- "Expected inlined-at fields to agree");
- if (FuncInfo.MF->useDebugInstrRef() && Op->isReg()) {
- // If using instruction referencing, produce this as a DBG_INSTR_REF,
- // to be later patched up by finalizeDebugInstrRefs. Tack a deref onto
- // the expression, we don't have an "indirect" flag in DBG_INSTR_REF.
- SmallVector<uint64_t, 3> Ops(
- {dwarf::DW_OP_LLVM_arg, 0, dwarf::DW_OP_deref});
- auto *NewExpr = DIExpression::prependOpcodes(DI->getExpression(), Ops);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD.getDL(),
- TII.get(TargetOpcode::DBG_INSTR_REF), /*IsIndirect*/ false, *Op,
- DI->getVariable(), NewExpr);
- } else {
- // A dbg.declare describes the address of a source variable, so lower it
- // into an indirect DBG_VALUE.
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD.getDL(),
- TII.get(TargetOpcode::DBG_VALUE), /*IsIndirect*/ true, *Op,
- DI->getVariable(), DI->getExpression());
- }
- } else {
- // We can't yet handle anything else here because it would require
- // generating code, thus altering codegen because of debug info.
- LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI
- << " (no materialized reg for address)\n");
- }
return true;
}
case Intrinsic::dbg_value: {
// This form of DBG_VALUE is target-independent.
const DbgValueInst *DI = cast<DbgValueInst>(II);
- const MCInstrDesc &II = TII.get(TargetOpcode::DBG_VALUE);
const Value *V = DI->getValue();
DIExpression *Expr = DI->getExpression();
DILocalVariable *Var = DI->getVariable();
+ if (DI->hasArgList())
+ // Signal that we don't have a location for this.
+ V = nullptr;
+
assert(Var->isValidLocationForIntrinsic(MIMD.getDL()) &&
"Expected inlined-at fields to agree");
- if (!V || isa<UndefValue>(V) || DI->hasArgList()) {
- // DI is either undef or cannot produce a valid DBG_VALUE, so produce an
- // undef DBG_VALUE to terminate any prior location.
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD.getDL(), II, false, 0U,
- Var, Expr);
- return true;
- }
- if (const auto *CI = dyn_cast<ConstantInt>(V)) {
- // See if there's an expression to constant-fold.
- if (Expr)
- std::tie(Expr, CI) = Expr->constantFold(CI);
- if (CI->getBitWidth() > 64)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addCImm(CI)
- .addImm(0U)
- .addMetadata(Var)
- .addMetadata(Expr);
- else
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addImm(CI->getZExtValue())
- .addImm(0U)
- .addMetadata(Var)
- .addMetadata(Expr);
- return true;
- }
- if (const auto *CF = dyn_cast<ConstantFP>(V)) {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addFPImm(CF)
- .addImm(0U)
- .addMetadata(Var)
- .addMetadata(Expr);
- return true;
- }
- if (const auto *Arg = dyn_cast<Argument>(V);
- Arg && Expr && Expr->isEntryValue()) {
- // As per the Verifier, this case is only valid for swift async Args.
- assert(Arg->hasAttribute(Attribute::AttrKind::SwiftAsync));
-
- Register Reg = getRegForValue(Arg);
- for (auto [PhysReg, VirtReg] : FuncInfo.RegInfo->liveins())
- if (Reg == VirtReg || Reg == PhysReg) {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD.getDL(), II,
- false /*IsIndirect*/, PhysReg, Var, Expr);
- return true;
- }
- LLVM_DEBUG(dbgs() << "Dropping dbg.value: expression is entry_value but "
- "couldn't find a physical register\n"
- << *DI << "\n");
- return true;
- }
- if (auto SI = FuncInfo.StaticAllocaMap.find(dyn_cast<AllocaInst>(V));
- SI != FuncInfo.StaticAllocaMap.end()) {
- MachineOperand FrameIndexOp = MachineOperand::CreateFI(SI->second);
- bool IsIndirect = false;
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD.getDL(), II, IsIndirect,
- FrameIndexOp, Var, Expr);
- return true;
- }
- if (Register Reg = lookUpRegForValue(V)) {
- // FIXME: This does not handle register-indirect values at offset 0.
- if (!FuncInfo.MF->useDebugInstrRef()) {
- bool IsIndirect = false;
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD.getDL(), II, IsIndirect,
- Reg, Var, Expr);
- return true;
- }
- // If using instruction referencing, produce this as a DBG_INSTR_REF,
- // to be later patched up by finalizeDebugInstrRefs.
- SmallVector<MachineOperand, 1> MOs({MachineOperand::CreateReg(
- /* Reg */ Reg, /* isDef */ false, /* isImp */ false,
- /* isKill */ false, /* isDead */ false,
- /* isUndef */ false, /* isEarlyClobber */ false,
- /* SubReg */ 0, /* isDebug */ true)});
- SmallVector<uint64_t, 2> Ops({dwarf::DW_OP_LLVM_arg, 0});
- auto *NewExpr = DIExpression::prependOpcodes(Expr, Ops);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD.getDL(),
- TII.get(TargetOpcode::DBG_INSTR_REF), /*IsIndirect*/ false, MOs,
- Var, NewExpr);
- return true;
- }
- // We don't know how to handle other cases, so we drop.
- LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI << "\n");
+ if (!lowerDbgValue(V, Expr, Var, MIMD.getDL()))
+ LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI << "\n");
+
return true;
}
case Intrinsic::dbg_label: {
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 3dc6e4bbcf46ba..53d4b0b984c0a4 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -1614,6 +1614,7 @@ void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
if (isFoldedOrDeadInstruction(Inst, *FuncInfo) ||
ElidedArgCopyInstrs.count(Inst)) {
--NumFastIselRemaining;
+ FastIS->handleDbgInfo(Inst);
continue;
}
@@ -1625,6 +1626,8 @@ void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
if (FastIS->selectInstruction(Inst)) {
--NumFastIselRemaining;
++NumFastIselSuccess;
+
+ FastIS->handleDbgInfo(Inst);
// If fast isel succeeded, skip over all the folded instructions, and
// then see if there is a load right before the selected instructions.
// Try to fold the load if so.
@@ -1640,6 +1643,7 @@ void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
// If we succeeded, don't re-select the load.
LLVM_DEBUG(dbgs()
<< "FastISel folded load: " << *BeforeInst << "\n");
+ FastIS->handleDbgInfo(BeforeInst);
BI = std::next(BasicBlock::const_iterator(BeforeInst));
--NumFastIselRemaining;
++NumFastIselSuccess;
diff --git a/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll b/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll
index 1d213945966a69..e1614d322b93d5 100644
--- a/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll
+++ b/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll
@@ -1,4 +1,5 @@
; RUN: llc -mtriple=aarch64 -O1 -opt-bisect-limit=2 -o - %s 2> /dev/null | FileCheck %s
+; RUN: llc --try-experimental-debuginfo-iterators -mtriple=aarch64 -O1 -opt-bisect-limit=2 -o - %s 2> /dev/null | FileCheck %s
define dso_local i32 @a() #0 !dbg !7 {
entry:
@@ -41,4 +42,4 @@ declare void @llvm.dbg.value(metadata, metadata, metadata) #2
!18 = distinct !{!18, !19, !20}
!19 = !DILocation(line: 3, column: 3, scope: !17)
!20 = !DILocation(line: 4, column: 5, scope: !17)
-
\ No newline at end of file
+
diff --git a/llvm/test/CodeGen/AArch64/fast-isel-dbg.ll b/llvm/test/CodeGen/AArch64/fast-isel-dbg.ll
index 602d454044d435..0c1a693089cb0d 100644
--- a/llvm/test/CodeGen/AArch64/fast-isel-dbg.ll
+++ b/llvm/test/CodeGen/AArch64/fast-isel-dbg.ll
@@ -1,4 +1,6 @@
; RUN: llc -o - %s -fast-isel -stop-before=finalize-isel | FileCheck %s
+; RUN: llc --try-experimental-debuginfo-iterators -o - %s -fast-isel -stop-before=finalize-isel | FileCheck %s
+
; Make sure fast-isel produces DBG_VALUE instructions even if no debug printer
; is scheduled because of -stop-before.
target triple="aarch64--"
diff --git a/llvm/test/CodeGen/AArch64/fastisel-debugvalue-undef.ll b/llvm/test/CodeGen/AArch64/fastisel-debugvalue-undef.ll
index cccede6a50459a..c30a01baedaf34 100644
--- a/llvm/test/CodeGen/AArch64/fastisel-debugvalue-undef.ll
+++ b/llvm/test/CodeGen/AArch64/fastisel-debugvalue-undef.ll
@@ -1,4 +1,5 @@
; RUN: llc -O0 -fast-isel=1 -o - -print-after="finalize-isel" %s 2>&1 | FileCheck %s
+; RUN: llc --try-experimental-debuginfo-iterators -O0 -fast-isel=1 -o - -print-after="finalize-isel" %s 2>&1 | FileCheck %s
; Check that we emit a DBG_VALUE for the `@llvm.dbg.value` which has `undef` has first arg.
diff --git a/llvm/test/CodeGen/ARM/debug-info-blocks.ll b/llvm/test/CodeGen/ARM/debug-info-blocks.ll
index 1c9ffb1775aa4f..8ef341faed6b73 100644
--- a/llvm/test/CodeGen/ARM/debug-info-blocks.ll
+++ b/llvm/test/CodeGen/ARM/debug-info-blocks.ll
@@ -1,4 +1,5 @@
; RUN: llc -filetype=obj -O0 < %s | llvm-dwarfdump -v - | FileCheck %s
+; RUN: llc --try-experimental-debuginfo-iterators -filetype=obj -O0 < %s | llvm-dwarfdump -v - | FileCheck %s
; debug_info content
; CHECK: DW_AT_name {{.*}} "foobar_func_block_invoke_0"
diff --git a/llvm/test/CodeGen/Generic/csw-debug-assert.ll b/llv...
[truncated]
|
jmorse
approved these changes
Jan 5, 2024
There was a problem hiding this comment.
This LGTM in principle -- I haven't compared the refactor'd code line-by-line, but it's a mechanical change, and the objective is to abstract the handling of debugging information from the type that it's stored in, which is fine. Two nits inline that want addressing before landing.
CC @felipepiovezan who IIRC touched this code last, so that it's on his radar. As mentioned, this is purely about abstracting the logic away from where the information is stored (intrinsics, or instruction-less debug-info).
Comment on lines
4
to
5
| ; RUN: llc -mtriple=thumbv7-apple-unknown-macho -O0 -filetype=obj -o - %s \ | ||
| ; RUN: | llvm-dwarfdump -v -debug-info - | FileCheck %s |
There was a problem hiding this comment.
Missed addition of --try-experimental-blah? Update of temp-file-to-pipe SGTM.
Comment on lines
+322
to
+323
| void handleDbgInfo(const Instruction *II); | ||
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The change is fairly mechanical:
FastISel::selectIntrinsicCall, which converts debug intrinsics into debug instructions, into functions (NFC).The test updates look the same as other RemoveDIs changes: re-run the tests with
--try-experimental-debuginfo-iterators, which checks the output is identical using the new debug info format (if it has been enabled in the cmake configuration).Depends on #76941 (otherwise some modified tests spuriously fail).