[llvm] [InstrRef] Preserve debug instr num in aarch64-ldst-opt. (PR #136009)
Shubham Sandeep Rastogi via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 29 14:54:24 PDT 2025
https://github.com/rastogishubham updated https://github.com/llvm/llvm-project/pull/136009
>From 9479004ff984306a5d0f6d13a41b40a338035576 Mon Sep 17 00:00:00 2001
From: Shubham Sandeep Rastogi <srastogi22 at apple.com>
Date: Wed, 16 Apr 2025 11:51:57 -0700
Subject: [PATCH 1/3] [InstrRef] Preserve debug instr num in aarch64-ldst-opt.
The aarch64-ldst-opt pass tries to merge two load instructions (LDR*) to
a load pair instruction (LDP*).
When merging the instructions, there is a case where one of the loads
would have to also be sign extended. In either case,
(sign extend or not), the pass needs to preserve the debug-instr-number
from the original loads to the load pair instruction to make sure
debug info isn't loast in the case where instruction referencing is
being used.
This patch addresses that problem.
---
.../AArch64/AArch64LoadStoreOptimizer.cpp | 81 +++++++++++++++++++
.../AArch64/aarch64-ldst-opt-instr-ref.mir | 79 ++++++++++++++++++
2 files changed, 160 insertions(+)
create mode 100644 llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir
diff --git a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
index 0e26005f6e6be..585394f8db837 100644
--- a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
+++ b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
@@ -964,6 +964,40 @@ static void updateDefinedRegisters(MachineInstr &MI, LiveRegUnits &Units,
Units.addReg(MOP.getReg());
}
+/// Find the DBG_INSTR_REF instruction that references the \p InstrNum
+static std::optional<MachineInstr *> findDebugInstrRef(MachineBasicBlock *MBB,
+ unsigned InstrNum) {
+
+ for (auto &MI : *MBB) {
+ if (MI.isDebugRef())
+ if (MI.getOperand(2).getInstrRefInstrIndex() == InstrNum)
+ return &MI;
+ }
+ return std::nullopt;
+}
+
+/// Set the correct debug-instr-number and the operand index in case of a merge.
+static void setDebugInstrNum(MachineBasicBlock::iterator OriginalInstr,
+ MachineInstrBuilder &MIB, unsigned InstrNumToFind,
+ unsigned InstrNumToSet, MachineBasicBlock *MBB) {
+
+ auto InstrRefMI = findDebugInstrRef(MBB, InstrNumToFind);
+ if (InstrRefMI) {
+ auto *MI = *InstrRefMI;
+ MI->getOperand(2).setInstrRefInstrIndex(InstrNumToSet);
+ // Set the Instruction Reference Op Index to be the same as the operand of
+ // the new merged pair instruction.
+ auto Reg = OriginalInstr->getOperand(0).getReg();
+ unsigned OperandNo = 0;
+ for (auto Op : MIB->operands()) {
+ if (Op.getReg() == Reg)
+ break;
+ OperandNo++;
+ }
+ MI->getOperand(2).setInstrRefOpIndex(OperandNo);
+ }
+}
+
MachineBasicBlock::iterator
AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
MachineBasicBlock::iterator Paired,
@@ -1226,6 +1260,30 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
.addImm(0)
.addImm(31);
(void)MIBSXTW;
+
+ if (I->peekDebugInstrNum()) {
+ auto InstrNum = I->peekDebugInstrNum();
+ // If I is the instruction which got sign extended, restore the debug
+ // instruction number from I to the SBFMXri instruction
+ if (DstRegX == I->getOperand(0).getReg())
+ MIBSXTW->setDebugInstrNum(InstrNum);
+ else {
+ MIB->setDebugInstrNum(InstrNum);
+ setDebugInstrNum(I, MIB, InstrNum, InstrNum, MBB);
+ }
+ }
+ if (Paired->peekDebugInstrNum()) {
+ auto InstrNum = Paired->peekDebugInstrNum();
+ // If Paired is the instruction which got sign extended, restore the debug
+ // instruction number from Paired to the SBFMXri instruction
+ if (DstRegX == Paired->getOperand(0).getReg())
+ MIBSXTW->setDebugInstrNum(InstrNum);
+ else {
+ MIB->setDebugInstrNum(InstrNum);
+ setDebugInstrNum(Paired, MIB, InstrNum, InstrNum, MBB);
+ }
+ }
+
LLVM_DEBUG(dbgs() << " Extend operand:\n ");
LLVM_DEBUG(((MachineInstr *)MIBSXTW)->print(dbgs()));
} else if (Opc == AArch64::LDR_ZXI || Opc == AArch64::STR_ZXI) {
@@ -1239,6 +1297,29 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
MOp1.setReg(AArch64::Q0 + (MOp1.getReg() - AArch64::Z0));
LLVM_DEBUG(((MachineInstr *)MIB)->print(dbgs()));
} else {
+
+ // Restore the debug instruction numbers to the merged instruction.
+ bool UseNewDebugInstrNum = false;
+ unsigned NewDebugInstrNum;
+ if (I->peekDebugInstrNum() && Paired->peekDebugInstrNum()) {
+ // Both instructions contain debug instruction numbers. We need to set a
+ // new instruction number for the paired instruction and update the
+ // DBG_INSTR_REFs that reference the instruction numbers of both
+ // instructions and update them.
+ NewDebugInstrNum = MIB->getDebugInstrNum();
+ UseNewDebugInstrNum = true;
+ }
+ if (I->peekDebugInstrNum())
+ setDebugInstrNum(
+ I, MIB, I->peekDebugInstrNum(),
+ UseNewDebugInstrNum ? NewDebugInstrNum : I->peekDebugInstrNum(), MBB);
+
+ if (Paired->peekDebugInstrNum())
+ setDebugInstrNum(Paired, MIB, Paired->peekDebugInstrNum(),
+ UseNewDebugInstrNum ? NewDebugInstrNum
+ : Paired->peekDebugInstrNum(),
+ MBB);
+
LLVM_DEBUG(((MachineInstr *)MIB)->print(dbgs()));
}
LLVM_DEBUG(dbgs() << "\n");
diff --git a/llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir b/llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir
new file mode 100644
index 0000000000000..a293d65be5b40
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir
@@ -0,0 +1,79 @@
+# RUN: llc -mtriple=aarch64-unknown-linux-gnu -o - %s -run-pass=aarch64-ldst-opt | FileCheck %s
+
+# This testcase was obtained by looking at FileCheck.cpp and reducing it down via llvm-reduce
+
+# The aarch64-ldst-opt pass tries to merge load instructions from LDR* to a load pair or LDP* instruction, in such a case, we must ensure that the debug-instr-number is properly preserved for instruction referencing.
+
+# Check that in the case of a sign extend, the debug instruction number is transferred to the sign extend instruction (SBFMXri in this case), whereas the LDP instruction gets the other debug instruction number for the load that doesn't get sign extended.
+
+# CHECK: $w[[REG1:[0-9]+]], renamable $w[[REG2:[0-9]+]] = LDPWi renamable ${{[a-z0-9]+}}, 0, debug-instr-number [[DBG_INSTR_NUM1:[0-9]+]]
+# CHECK-NEXT: $w[[REG1]] = KILL $w[[REG1]], implicit-def $x[[REG1]]
+# CHECK-NEXT: $x[[REG1]] = SBFMXri $x[[REG1]], 0, 31, debug-instr-number [[DBG_INSTR_NUM2:[0-9]+]]
+# CHECK-NEXT: DBG_INSTR_REF ![[DBG1:[0-9]+]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref([[DBG_INSTR_NUM1]], 1), debug-location ![[DBG2:[0-9]+]]
+# CHECK-NEXT: DBG_INSTR_REF ![[DBG1]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref([[DBG_INSTR_NUM2]], 0), debug-location ![[DBG2]]
+
+# Check that in the case there is no sign extend, the LDP instruction gets a new debug instruction number and both the DBG_INSTR_REFs use the new instruction number.
+
+# CHECK: renamable $x[[REG3:[0-9]+]], renamable $x[[REG4:[0-9]+]] = LDPXi renamable ${{[a-z0-9]+}}, 0, debug-instr-number [[DBG_INSTR_NUM3:[0-9]+]]
+# CHECK-NEXT: DBG_INSTR_REF ![[DBG3:[0-9]+]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref([[DBG_INSTR_NUM3]], 1), debug-location ![[DBG4:[0-9]+]]
+# CHECK-NEXT: DBG_INSTR_REF ![[DBG3]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref([[DBG_INSTR_NUM3]], 0), debug-location ![[DBG4]]
+
+--- |
+ define i64 @_ZNK4llvm9StringRef4sizeEv(ptr readonly captures(none) %this) local_unnamed_addr #0 {
+ entry:
+ %Length = getelementptr i8, ptr %this, i64 8
+ %0 = load i64, ptr %Length, align 4
+ ret i64 %0
+ }
+ define ptr @_ZNK4llvm9StringRef4dataEv(ptr readonly captures(none) %this) local_unnamed_addr #0 {
+ entry:
+ %0 = load ptr, ptr %this, align 8
+ ret ptr %0
+ }
+ define void @_ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE(ptr readonly captures(none) %agg.result) local_unnamed_addr !dbg !3 {
+ %call1541 = load volatile ptr, ptr null, align 4294967296, !dbg !9
+ %FullMatch.sroa.1.0.agg.result.sroa_idx = getelementptr inbounds nuw i8, ptr %agg.result, i64 8
+ ret void
+ }
+ define void @_ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE2(ptr readonly captures(none) %agg.result) local_unnamed_addr !dbg !10 {
+ %call1541 = load volatile ptr, ptr null, align 4294967296, !dbg !11
+ %FullMatch.sroa.1.0.agg.result.sroa_idx = getelementptr inbounds nuw i8, ptr %agg.result, i64 8
+ ret void
+ }
+ !llvm.module.flags = !{!0}
+ !llvm.dbg.cu = !{!1}
+ !0 = !{i32 2, !"Debug Info Version", i32 3}
+ !1 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !2, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, sdk: "MacOSX15.3.sdk")
+ !2 = !DIFile(filename: "/Users/shubhamrastogi/Development/llvm-project-instr-ref/llvm-project/llvm/lib/FileCheck/FileCheck.cpp", directory: "/Users/shubhamrastogi/Development/llvm-project-instr-ref/llvm-project/build-baseline-stage2", checksumkind: CSK_MD5, checksum: "ac1d2352ab68b965fe7993c780cf92d7")
+ !3 = distinct !DISubprogram(scope: null, type: !4, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !6)
+ !4 = distinct !DISubroutineType(types: !5)
+ !5 = !{}
+ !6 = !{!7}
+ !7 = !DILocalVariable(name: "FullMatch", scope: !3, line: 1152, type: !8)
+ !8 = distinct !DICompositeType(tag: DW_TAG_class_type, size: 128, identifier: "_ZTSN4llvm9StringRefE")
+ !9 = !DILocation(line: 0, scope: !3)
+ !10 = distinct !DISubprogram(scope: null, type: !4, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !12)
+ !11 = !DILocation(line: 0, scope: !10)
+ !12 = !{!13}
+ !13 = !DILocalVariable(name: "FullMatch", scope: !10, line: 1152, type: !14)
+ !14 = distinct !DICompositeType(tag: DW_TAG_class_type, size: 128, identifier: "_ZTSN4llvm9StringRefE")
+
+name: _ZNK4llvm9StringRef4sizeEv
+---
+name: _ZNK4llvm9StringRef4dataEv
+...
+name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE
+body: |
+ bb.0 (%ir-block.0):
+ renamable $w1 = LDRWui renamable $x0, 1, debug-instr-number 1 :: (load (s64) from %ir.FullMatch.sroa.1.0.agg.result.sroa_idx, align 1)
+ DBG_INSTR_REF !7, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref(1, 0), debug-location !9
+ renamable $x0 = LDRSWui killed renamable $x0, 0, debug-instr-number 2 :: (load (s64) from %ir.agg.result)
+ DBG_INSTR_REF !7, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref(2, 0), debug-location !9
+...
+name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE2
+body: |
+ bb.0 (%ir-block.0):
+ renamable $x1 = LDRXui renamable $x0, 1, debug-instr-number 1 :: (load (s64) from %ir.FullMatch.sroa.1.0.agg.result.sroa_idx, align 1)
+ DBG_INSTR_REF !13, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref(1, 0), debug-location !11
+ renamable $x0 = LDRXui killed renamable $x0, 0, debug-instr-number 2 :: (load (s64) from %ir.agg.result)
+ DBG_INSTR_REF !13, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref(2, 0), debug-location !11
>From 92a561a40bb6a8542e5e02caae619c89ab6e0fd6 Mon Sep 17 00:00:00 2001
From: Shubham Sandeep Rastogi <srastogi22 at apple.com>
Date: Fri, 18 Apr 2025 18:42:19 -0700
Subject: [PATCH 2/3] addressed review comments
---
.../AArch64/AArch64LoadStoreOptimizer.cpp | 186 +++++++++++-------
.../AArch64/aarch64-ldst-opt-instr-ref.mir | 26 ++-
2 files changed, 139 insertions(+), 73 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
index 585394f8db837..157765eb2ad08 100644
--- a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
+++ b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
@@ -964,38 +964,26 @@ static void updateDefinedRegisters(MachineInstr &MI, LiveRegUnits &Units,
Units.addReg(MOP.getReg());
}
-/// Find the DBG_INSTR_REF instruction that references the \p InstrNum
-static std::optional<MachineInstr *> findDebugInstrRef(MachineBasicBlock *MBB,
- unsigned InstrNum) {
-
- for (auto &MI : *MBB) {
- if (MI.isDebugRef())
- if (MI.getOperand(2).getInstrRefInstrIndex() == InstrNum)
- return &MI;
+/// This function will add a new entry into the debugValueSubstitutions table
+/// when two instruction have been merged into a new one represented by \p
+/// MergedInstr.
+static void addDebugSubstitutionsToTable(MachineFunction *MF,
+ unsigned InstrNumToSet,
+ MachineInstr &OriginalInstr,
+ MachineInstr &MergedInstr) {
+
+ // Figure out the Operand Index of the destination register of the
+ // OriginalInstr in the new MergedInstr.
+ auto Reg = OriginalInstr.getOperand(0).getReg();
+ unsigned OperandNo = 0;
+ for (auto Op : MergedInstr.operands()) {
+ if (Op.getReg() == Reg)
+ break;
+ OperandNo++;
}
- return std::nullopt;
-}
-/// Set the correct debug-instr-number and the operand index in case of a merge.
-static void setDebugInstrNum(MachineBasicBlock::iterator OriginalInstr,
- MachineInstrBuilder &MIB, unsigned InstrNumToFind,
- unsigned InstrNumToSet, MachineBasicBlock *MBB) {
-
- auto InstrRefMI = findDebugInstrRef(MBB, InstrNumToFind);
- if (InstrRefMI) {
- auto *MI = *InstrRefMI;
- MI->getOperand(2).setInstrRefInstrIndex(InstrNumToSet);
- // Set the Instruction Reference Op Index to be the same as the operand of
- // the new merged pair instruction.
- auto Reg = OriginalInstr->getOperand(0).getReg();
- unsigned OperandNo = 0;
- for (auto Op : MIB->operands()) {
- if (Op.getReg() == Reg)
- break;
- OperandNo++;
- }
- MI->getOperand(2).setInstrRefOpIndex(OperandNo);
- }
+ MF->makeDebugValueSubstitution({OriginalInstr.peekDebugInstrNum(), 0},
+ {InstrNumToSet, OperandNo});
}
MachineBasicBlock::iterator
@@ -1261,26 +1249,75 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
.addImm(31);
(void)MIBSXTW;
+ // In the case of a sign-extend, where we have something like:
+ // debugValueSubstitutions:[]
+ // $w1 = LDRWui $x0, 1, debug-instr-number 1
+ // DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
+ // $x0 = LDRSWui $x0, 0, debug-instr-number 2
+ // DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
+
+ // It will be converted to:
+ // debugValueSubstitutions:[]
+ // $w0, $w1 = LDPWi $x0, 0
+ // $w0 = KILL $w0, implicit-def $x0
+ // $x0 = SBFMXri $x0, 0, 31
+ // DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
+ // DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
+
+ // $x0 is where the final value is stored, so the sign extend (SBFMXri)
+ // instruction contains the final value we care about we give it a new
+ // debug-instr-number 3. Whereas, $w1 contains the final value that we care
+ // about, therefore the LDP instruction is also given a new
+ // debug-instr-number 4. We have to add these subsitutions to the
+ // debugValueSubstitutions table. However, we also have to ensure that the
+ // OpIndex that pointed to debug-instr-number 1 gets updated to 1, because
+ // $w1 is the second operand of the LDP instruction.
+
+ // We want the final result to look like:
+ // debugValueSubstitutions:
+ // - { srcinst: 1, srcop: 0, dstinst: 4, dstop: 1, subreg: 0 }
+ // - { srcinst: 2, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
+ // $w0, $w1 = LDPWi $x0, 0, debug-instr-number 4
+ // $w0 = KILL $w0, implicit-def $x0
+ // $x0 = SBFMXri $x0, 0, 31, debug-instr-number 3
+ // DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
+ // DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
+
if (I->peekDebugInstrNum()) {
- auto InstrNum = I->peekDebugInstrNum();
- // If I is the instruction which got sign extended, restore the debug
- // instruction number from I to the SBFMXri instruction
- if (DstRegX == I->getOperand(0).getReg())
- MIBSXTW->setDebugInstrNum(InstrNum);
- else {
- MIB->setDebugInstrNum(InstrNum);
- setDebugInstrNum(I, MIB, InstrNum, InstrNum, MBB);
+ // If I is the instruction which got sign extended and has a
+ // debug-instr-number, give the SBFMXri instruction a new
+ // debug-instr-number, and update the debugValueSubstitutions table with
+ // the new debug-instr-number and OpIndex pair. Otherwise, give the Merged
+ // instruction a new debug-instr-number, and update the
+ // debugValueSubstitutions table with the new debug-instr-number and
+ // OpIndex pair.
+ unsigned NewInstrNum;
+ if (DstRegX == I->getOperand(0).getReg()) {
+ NewInstrNum = MIBSXTW->getDebugInstrNum();
+ addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *I,
+ *MIBSXTW);
+ } else {
+ NewInstrNum = MIB->getDebugInstrNum();
+ addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *I, *MIB);
}
}
if (Paired->peekDebugInstrNum()) {
- auto InstrNum = Paired->peekDebugInstrNum();
- // If Paired is the instruction which got sign extended, restore the debug
- // instruction number from Paired to the SBFMXri instruction
- if (DstRegX == Paired->getOperand(0).getReg())
- MIBSXTW->setDebugInstrNum(InstrNum);
- else {
- MIB->setDebugInstrNum(InstrNum);
- setDebugInstrNum(Paired, MIB, InstrNum, InstrNum, MBB);
+ // If Paired is the instruction which got sign extended and has a
+ // debug-instr-number, give the SBFMXri instruction a new
+ // debug-instr-number, and update the debugValueSubstitutions table with
+ // the new debug-instr-number and OpIndex pair. Otherwise, give the Merged
+ // instruction a new debug-instr-number, and update the
+ // debugValueSubstitutions table with the new debug-instr-number and
+ // OpIndex pair.
+ unsigned NewInstrNum;
+ if (DstRegX == Paired->getOperand(0).getReg()) {
+ NewInstrNum = MIBSXTW->getDebugInstrNum();
+ addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *Paired,
+ *MIBSXTW);
+ } else {
+ NewInstrNum = MIB->getDebugInstrNum();
+ addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *Paired,
+ *MIB);
}
}
@@ -1298,27 +1335,44 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
LLVM_DEBUG(((MachineInstr *)MIB)->print(dbgs()));
} else {
- // Restore the debug instruction numbers to the merged instruction.
- bool UseNewDebugInstrNum = false;
- unsigned NewDebugInstrNum;
- if (I->peekDebugInstrNum() && Paired->peekDebugInstrNum()) {
- // Both instructions contain debug instruction numbers. We need to set a
- // new instruction number for the paired instruction and update the
- // DBG_INSTR_REFs that reference the instruction numbers of both
- // instructions and update them.
- NewDebugInstrNum = MIB->getDebugInstrNum();
- UseNewDebugInstrNum = true;
+ // In the case that the merge doesn't result in a sign-extend, if we have
+ // something like:
+ // debugValueSubstitutions:[]
+ // $x1 = LDRXui $x0, 1, debug-instr-number 1
+ // DBG_INSTR_REF !13, dbg-instr-ref(1, 0), debug-location !11
+ // $x0 = LDRXui killed $x0, 0, debug-instr-number 2
+ // DBG_INSTR_REF !14, dbg-instr-ref(2, 0), debug-location !11
+
+ // It will be converted to:
+ // debugValueSubstitutions: []
+ // $x0, $x1 = LDPXi $x0, 0
+ // DBG_INSTR_REF !12, dbg-instr-ref(1, 0), debug-location !14
+ // DBG_INSTR_REF !13, dbg-instr-ref(2, 0), debug-location !14
+
+ // Here all that needs to be done is, that the LDP instruction needs to be
+ // updated with a new debug-instr-number, we then need to add entries into
+ // the debugSubstitutions table to map the old instr-refs to the new ones.
+
+ // We want the final result to look like:
+ // debugValueSubstitutions:
+ // - { srcinst: 1, srcop: 0, dstinst: 3, dstop: 1, subreg: 0 }
+ // - { srcinst: 2, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
+ // $x0, $x1 = LDPXi $x0, 0, debug-instr-number 3
+ // DBG_INSTR_REF !12, dbg-instr-ref(1, 0), debug-location !14
+ // DBG_INSTR_REF !12, dbg-instr-ref(2, 0), debug-location !14
+
+ // Assign new DebugInstrNum to the Paired instruction.
+
+ if (I->peekDebugInstrNum()) {
+ unsigned NewDebugInstrNum = MIB->getDebugInstrNum();
+ addDebugSubstitutionsToTable(MBB->getParent(), NewDebugInstrNum, *I,
+ *MIB);
+ }
+ if (Paired->peekDebugInstrNum()) {
+ unsigned NewDebugInstrNum = MIB->getDebugInstrNum();
+ addDebugSubstitutionsToTable(MBB->getParent(), NewDebugInstrNum, *Paired,
+ *MIB);
}
- if (I->peekDebugInstrNum())
- setDebugInstrNum(
- I, MIB, I->peekDebugInstrNum(),
- UseNewDebugInstrNum ? NewDebugInstrNum : I->peekDebugInstrNum(), MBB);
-
- if (Paired->peekDebugInstrNum())
- setDebugInstrNum(Paired, MIB, Paired->peekDebugInstrNum(),
- UseNewDebugInstrNum ? NewDebugInstrNum
- : Paired->peekDebugInstrNum(),
- MBB);
LLVM_DEBUG(((MachineInstr *)MIB)->print(dbgs()));
}
diff --git a/llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir b/llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir
index a293d65be5b40..5a6b4c75a6ecf 100644
--- a/llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir
+++ b/llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir
@@ -6,17 +6,27 @@
# Check that in the case of a sign extend, the debug instruction number is transferred to the sign extend instruction (SBFMXri in this case), whereas the LDP instruction gets the other debug instruction number for the load that doesn't get sign extended.
-# CHECK: $w[[REG1:[0-9]+]], renamable $w[[REG2:[0-9]+]] = LDPWi renamable ${{[a-z0-9]+}}, 0, debug-instr-number [[DBG_INSTR_NUM1:[0-9]+]]
+# CHECK-LABEL: name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE
+# CHECK: debugValueSubstitutions:
+# CHECK-NEXT: - { srcinst: [[DBG_INSTR_NUM1:[0-9+]]], srcop: [[DBG_INSTR_OP1:[0-9+]]], dstinst: [[DBG_INSTR_NUM2:[0-9+]]], dstop: 1, subreg: 0 }
+# CHECK-NEXT: - { srcinst: [[DBG_INSTR_NUM3:[0-9+]]], srcop: [[DBG_INSTR_OP2:[0-9+]]], dstinst: [[DBG_INSTR_NUM4:[0-9+]]], dstop: 0, subreg: 0 }
+
+# CHECK: $w[[REG1:[0-9+]]], renamable $w[[REG2:[0-9+]]] = LDPWi renamable $x[[REG1]], 0, debug-instr-number [[DBG_INSTR_NUM2]]
# CHECK-NEXT: $w[[REG1]] = KILL $w[[REG1]], implicit-def $x[[REG1]]
-# CHECK-NEXT: $x[[REG1]] = SBFMXri $x[[REG1]], 0, 31, debug-instr-number [[DBG_INSTR_NUM2:[0-9]+]]
-# CHECK-NEXT: DBG_INSTR_REF ![[DBG1:[0-9]+]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref([[DBG_INSTR_NUM1]], 1), debug-location ![[DBG2:[0-9]+]]
-# CHECK-NEXT: DBG_INSTR_REF ![[DBG1]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref([[DBG_INSTR_NUM2]], 0), debug-location ![[DBG2]]
+# CHECK-NEXT: $x[[REG1]] = SBFMXri $x[[REG1]], 0, 31, debug-instr-number [[DBG_INSTR_NUM4]]
+# CHECK-NEXT: DBG_INSTR_REF !{{[0-9+]}}, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref([[DBG_INSTR_NUM1]], [[DBG_INSTR_OP1]]), debug-location !{{[0-9+]}}
+# CHECK-NEXT: DBG_INSTR_REF !7, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref([[DBG_INSTR_NUM3]], [[DBG_INSTR_OP2]]), debug-location !{{[0-9+]}}
# Check that in the case there is no sign extend, the LDP instruction gets a new debug instruction number and both the DBG_INSTR_REFs use the new instruction number.
-# CHECK: renamable $x[[REG3:[0-9]+]], renamable $x[[REG4:[0-9]+]] = LDPXi renamable ${{[a-z0-9]+}}, 0, debug-instr-number [[DBG_INSTR_NUM3:[0-9]+]]
-# CHECK-NEXT: DBG_INSTR_REF ![[DBG3:[0-9]+]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref([[DBG_INSTR_NUM3]], 1), debug-location ![[DBG4:[0-9]+]]
-# CHECK-NEXT: DBG_INSTR_REF ![[DBG3]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref([[DBG_INSTR_NUM3]], 0), debug-location ![[DBG4]]
+# CHECK-LABEL: name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE2
+# CHECK: debugValueSubstitutions:
+# CHECK-NEXT: - { srcinst: [[DBG_INSTR_NUM5:[0-9+]]], srcop: [[DBG_INSTR_OP3:[0-9+]]], dstinst: [[DBG_INSTR_NUM6:[0-9+]]], dstop: 1, subreg: 0 }
+# CHECK-NEXT: - { srcinst: [[DBG_INSTR_NUM7:[0-9+]]], srcop: [[DBG_INSTR_OP4:[0-9+]]], dstinst: [[DBG_INSTR_NUM6]], dstop: 0, subreg: 0 }
+
+# CHECK: renamable $x[[REG3:[0-9+]]], renamable $x[[REG4:[0-9+]]] = LDPXi renamable $x[[REG3]], 0, debug-instr-number [[DBG_INSTR_NUM6]]
+# CHECK-NEXT: DBG_INSTR_REF !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref([[DBG_INSTR_NUM5]], [[DBG_INSTR_OP3]]), debug-location !14
+# CHECK-NEXT: DBG_INSTR_REF !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref([[DBG_INSTR_NUM7]], [[DBG_INSTR_OP4]]), debug-location !14
--- |
define i64 @_ZNK4llvm9StringRef4sizeEv(ptr readonly captures(none) %this) local_unnamed_addr #0 {
@@ -63,6 +73,7 @@ name: _ZNK4llvm9StringRef4sizeEv
name: _ZNK4llvm9StringRef4dataEv
...
name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE
+debugValueSubstitutions: []
body: |
bb.0 (%ir-block.0):
renamable $w1 = LDRWui renamable $x0, 1, debug-instr-number 1 :: (load (s64) from %ir.FullMatch.sroa.1.0.agg.result.sroa_idx, align 1)
@@ -71,6 +82,7 @@ body: |
DBG_INSTR_REF !7, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref(2, 0), debug-location !9
...
name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE2
+debugValueSubstitutions: []
body: |
bb.0 (%ir-block.0):
renamable $x1 = LDRXui renamable $x0, 1, debug-instr-number 1 :: (load (s64) from %ir.FullMatch.sroa.1.0.agg.result.sroa_idx, align 1)
>From 497521c3399bd8c95975f6f4ada2c633da73971a Mon Sep 17 00:00:00 2001
From: Shubham Sandeep Rastogi <srastogi22 at apple.com>
Date: Tue, 29 Apr 2025 14:52:20 -0700
Subject: [PATCH 3/3] Addressed some more review comments
---
.../AArch64/AArch64LoadStoreOptimizer.cpp | 39 ++++++++++---------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
index 157765eb2ad08..ba3ffc2f6eb1f 100644
--- a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
+++ b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
@@ -976,14 +976,18 @@ static void addDebugSubstitutionsToTable(MachineFunction *MF,
// OriginalInstr in the new MergedInstr.
auto Reg = OriginalInstr.getOperand(0).getReg();
unsigned OperandNo = 0;
- for (auto Op : MergedInstr.operands()) {
- if (Op.getReg() == Reg)
+ bool RegFound = false;
+ for (const auto Op : MergedInstr.operands()) {
+ if (Op.getReg() == Reg) {
+ RegFound = true;
break;
+ }
OperandNo++;
}
- MF->makeDebugValueSubstitution({OriginalInstr.peekDebugInstrNum(), 0},
- {InstrNumToSet, OperandNo});
+ if (RegFound)
+ MF->makeDebugValueSubstitution({OriginalInstr.peekDebugInstrNum(), 0},
+ {InstrNumToSet, OperandNo});
}
MachineBasicBlock::iterator
@@ -1264,15 +1268,6 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
// DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
// DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
- // $x0 is where the final value is stored, so the sign extend (SBFMXri)
- // instruction contains the final value we care about we give it a new
- // debug-instr-number 3. Whereas, $w1 contains the final value that we care
- // about, therefore the LDP instruction is also given a new
- // debug-instr-number 4. We have to add these subsitutions to the
- // debugValueSubstitutions table. However, we also have to ensure that the
- // OpIndex that pointed to debug-instr-number 1 gets updated to 1, because
- // $w1 is the second operand of the LDP instruction.
-
// We want the final result to look like:
// debugValueSubstitutions:
// - { srcinst: 1, srcop: 0, dstinst: 4, dstop: 1, subreg: 0 }
@@ -1283,6 +1278,15 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
// DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
// DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
+ // $x0 is where the final value is stored, so the sign extend (SBFMXri)
+ // instruction contains the final value we care about we give it a new
+ // debug-instr-number 3. Whereas, $w1 contains the final value that we care
+ // about, therefore the LDP instruction is also given a new
+ // debug-instr-number 4. We have to add these subsitutions to the
+ // debugValueSubstitutions table. However, we also have to ensure that the
+ // OpIndex that pointed to debug-instr-number 1 gets updated to 1, because
+ // $w1 is the second operand of the LDP instruction.
+
if (I->peekDebugInstrNum()) {
// If I is the instruction which got sign extended and has a
// debug-instr-number, give the SBFMXri instruction a new
@@ -1349,10 +1353,6 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
// DBG_INSTR_REF !12, dbg-instr-ref(1, 0), debug-location !14
// DBG_INSTR_REF !13, dbg-instr-ref(2, 0), debug-location !14
- // Here all that needs to be done is, that the LDP instruction needs to be
- // updated with a new debug-instr-number, we then need to add entries into
- // the debugSubstitutions table to map the old instr-refs to the new ones.
-
// We want the final result to look like:
// debugValueSubstitutions:
// - { srcinst: 1, srcop: 0, dstinst: 3, dstop: 1, subreg: 0 }
@@ -1361,8 +1361,11 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
// DBG_INSTR_REF !12, dbg-instr-ref(1, 0), debug-location !14
// DBG_INSTR_REF !12, dbg-instr-ref(2, 0), debug-location !14
- // Assign new DebugInstrNum to the Paired instruction.
+ // Here all that needs to be done is, that the LDP instruction needs to be
+ // updated with a new debug-instr-number, we then need to add entries into
+ // the debugSubstitutions table to map the old instr-refs to the new ones.
+ // Assign new DebugInstrNum to the Paired instruction.
if (I->peekDebugInstrNum()) {
unsigned NewDebugInstrNum = MIB->getDebugInstrNum();
addDebugSubstitutionsToTable(MBB->getParent(), NewDebugInstrNum, *I,
More information about the llvm-commits
mailing list