Skip to content

Commit 1149dcb

Browse files
rastogishubhamGeorgeARM
rastogishubham
authored and
GeorgeARM
committed
[InstrRef] Preserve debug instr num in aarch64-ldst-opt. (llvm#136009)
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 lost in the case where instruction referencing is being used. For example: We can have something like this: ``` 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 ``` This would be changed 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 ``` In this case, we need to create a new debug instruction number for the `LDP` instruction, we then need to add entries into the debugSubstitutions table to map the old instr-refs to the new ones. After this patch, the result will be: ``` 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 ``` However, this is not all, we also can have a case where there is a sign-extend involved, let's look at the case: ``` 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 ``` This will become: ``` 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. The result after the patch looks 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 ``` This patch addresses that problem.
1 parent 83a2136 commit 1149dcb

File tree

2 files changed

+229
-0
lines changed

2 files changed

+229
-0
lines changed

‎llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,6 +964,32 @@ static void updateDefinedRegisters(MachineInstr &MI, LiveRegUnits &Units,
964964
Units.addReg(MOP.getReg());
965965
}
966966

967+
/// This function will add a new entry into the debugValueSubstitutions table
968+
/// when two instruction have been merged into a new one represented by \p
969+
/// MergedInstr.
970+
static void addDebugSubstitutionsToTable(MachineFunction *MF,
971+
unsigned InstrNumToSet,
972+
MachineInstr &OriginalInstr,
973+
MachineInstr &MergedInstr) {
974+
975+
// Figure out the Operand Index of the destination register of the
976+
// OriginalInstr in the new MergedInstr.
977+
auto Reg = OriginalInstr.getOperand(0).getReg();
978+
unsigned OperandNo = 0;
979+
bool RegFound = false;
980+
for (const auto Op : MergedInstr.operands()) {
981+
if (Op.getReg() == Reg) {
982+
RegFound = true;
983+
break;
984+
}
985+
OperandNo++;
986+
}
987+
988+
if (RegFound)
989+
MF->makeDebugValueSubstitution({OriginalInstr.peekDebugInstrNum(), 0},
990+
{InstrNumToSet, OperandNo});
991+
}
992+
967993
MachineBasicBlock::iterator
968994
AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
969995
MachineBasicBlock::iterator Paired,
@@ -1226,6 +1252,79 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
12261252
.addImm(0)
12271253
.addImm(31);
12281254
(void)MIBSXTW;
1255+
1256+
// In the case of a sign-extend, where we have something like:
1257+
// debugValueSubstitutions:[]
1258+
// $w1 = LDRWui $x0, 1, debug-instr-number 1
1259+
// DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
1260+
// $x0 = LDRSWui $x0, 0, debug-instr-number 2
1261+
// DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
1262+
1263+
// It will be converted to:
1264+
// debugValueSubstitutions:[]
1265+
// $w0, $w1 = LDPWi $x0, 0
1266+
// $w0 = KILL $w0, implicit-def $x0
1267+
// $x0 = SBFMXri $x0, 0, 31
1268+
// DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
1269+
// DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
1270+
1271+
// We want the final result to look like:
1272+
// debugValueSubstitutions:
1273+
// - { srcinst: 1, srcop: 0, dstinst: 4, dstop: 1, subreg: 0 }
1274+
// - { srcinst: 2, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
1275+
// $w0, $w1 = LDPWi $x0, 0, debug-instr-number 4
1276+
// $w0 = KILL $w0, implicit-def $x0
1277+
// $x0 = SBFMXri $x0, 0, 31, debug-instr-number 3
1278+
// DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
1279+
// DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
1280+
1281+
// $x0 is where the final value is stored, so the sign extend (SBFMXri)
1282+
// instruction contains the final value we care about we give it a new
1283+
// debug-instr-number 3. Whereas, $w1 contains the final value that we care
1284+
// about, therefore the LDP instruction is also given a new
1285+
// debug-instr-number 4. We have to add these subsitutions to the
1286+
// debugValueSubstitutions table. However, we also have to ensure that the
1287+
// OpIndex that pointed to debug-instr-number 1 gets updated to 1, because
1288+
// $w1 is the second operand of the LDP instruction.
1289+
1290+
if (I->peekDebugInstrNum()) {
1291+
// If I is the instruction which got sign extended and has a
1292+
// debug-instr-number, give the SBFMXri instruction a new
1293+
// debug-instr-number, and update the debugValueSubstitutions table with
1294+
// the new debug-instr-number and OpIndex pair. Otherwise, give the Merged
1295+
// instruction a new debug-instr-number, and update the
1296+
// debugValueSubstitutions table with the new debug-instr-number and
1297+
// OpIndex pair.
1298+
unsigned NewInstrNum;
1299+
if (DstRegX == I->getOperand(0).getReg()) {
1300+
NewInstrNum = MIBSXTW->getDebugInstrNum();
1301+
addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *I,
1302+
*MIBSXTW);
1303+
} else {
1304+
NewInstrNum = MIB->getDebugInstrNum();
1305+
addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *I, *MIB);
1306+
}
1307+
}
1308+
if (Paired->peekDebugInstrNum()) {
1309+
// If Paired is the instruction which got sign extended and has a
1310+
// debug-instr-number, give the SBFMXri instruction a new
1311+
// debug-instr-number, and update the debugValueSubstitutions table with
1312+
// the new debug-instr-number and OpIndex pair. Otherwise, give the Merged
1313+
// instruction a new debug-instr-number, and update the
1314+
// debugValueSubstitutions table with the new debug-instr-number and
1315+
// OpIndex pair.
1316+
unsigned NewInstrNum;
1317+
if (DstRegX == Paired->getOperand(0).getReg()) {
1318+
NewInstrNum = MIBSXTW->getDebugInstrNum();
1319+
addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *Paired,
1320+
*MIBSXTW);
1321+
} else {
1322+
NewInstrNum = MIB->getDebugInstrNum();
1323+
addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *Paired,
1324+
*MIB);
1325+
}
1326+
}
1327+
12291328
LLVM_DEBUG(dbgs() << " Extend operand:\n ");
12301329
LLVM_DEBUG(((MachineInstr *)MIBSXTW)->print(dbgs()));
12311330
} else if (Opc == AArch64::LDR_ZXI || Opc == AArch64::STR_ZXI) {
@@ -1239,6 +1338,45 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
12391338
MOp1.setReg(AArch64::Q0 + (MOp1.getReg() - AArch64::Z0));
12401339
LLVM_DEBUG(((MachineInstr *)MIB)->print(dbgs()));
12411340
} else {
1341+
1342+
// In the case that the merge doesn't result in a sign-extend, if we have
1343+
// something like:
1344+
// debugValueSubstitutions:[]
1345+
// $x1 = LDRXui $x0, 1, debug-instr-number 1
1346+
// DBG_INSTR_REF !13, dbg-instr-ref(1, 0), debug-location !11
1347+
// $x0 = LDRXui killed $x0, 0, debug-instr-number 2
1348+
// DBG_INSTR_REF !14, dbg-instr-ref(2, 0), debug-location !11
1349+
1350+
// It will be converted to:
1351+
// debugValueSubstitutions: []
1352+
// $x0, $x1 = LDPXi $x0, 0
1353+
// DBG_INSTR_REF !12, dbg-instr-ref(1, 0), debug-location !14
1354+
// DBG_INSTR_REF !13, dbg-instr-ref(2, 0), debug-location !14
1355+
1356+
// We want the final result to look like:
1357+
// debugValueSubstitutions:
1358+
// - { srcinst: 1, srcop: 0, dstinst: 3, dstop: 1, subreg: 0 }
1359+
// - { srcinst: 2, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
1360+
// $x0, $x1 = LDPXi $x0, 0, debug-instr-number 3
1361+
// DBG_INSTR_REF !12, dbg-instr-ref(1, 0), debug-location !14
1362+
// DBG_INSTR_REF !12, dbg-instr-ref(2, 0), debug-location !14
1363+
1364+
// Here all that needs to be done is, that the LDP instruction needs to be
1365+
// updated with a new debug-instr-number, we then need to add entries into
1366+
// the debugSubstitutions table to map the old instr-refs to the new ones.
1367+
1368+
// Assign new DebugInstrNum to the Paired instruction.
1369+
if (I->peekDebugInstrNum()) {
1370+
unsigned NewDebugInstrNum = MIB->getDebugInstrNum();
1371+
addDebugSubstitutionsToTable(MBB->getParent(), NewDebugInstrNum, *I,
1372+
*MIB);
1373+
}
1374+
if (Paired->peekDebugInstrNum()) {
1375+
unsigned NewDebugInstrNum = MIB->getDebugInstrNum();
1376+
addDebugSubstitutionsToTable(MBB->getParent(), NewDebugInstrNum, *Paired,
1377+
*MIB);
1378+
}
1379+
12421380
LLVM_DEBUG(((MachineInstr *)MIB)->print(dbgs()));
12431381
}
12441382
LLVM_DEBUG(dbgs() << "\n");

‎llvm/test/CodeGen/AArch64/aarch64-ldst-opt-instr-ref.mir‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
# RUN: llc -mtriple=aarch64-unknown-linux-gnu -o - %s -run-pass=aarch64-ldst-opt | FileCheck %s
2+
3+
# This testcase was obtained by looking at FileCheck.cpp and reducing it down via llvm-reduce
4+
5+
# 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.
6+
7+
# 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.
8+
9+
# CHECK-LABEL: name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE
10+
# CHECK: debugValueSubstitutions:
11+
# CHECK-NEXT: - { srcinst: [[DBG_INSTR_NUM1:[0-9+]]], srcop: [[DBG_INSTR_OP1:[0-9+]]], dstinst: [[DBG_INSTR_NUM2:[0-9+]]], dstop: 1, subreg: 0 }
12+
# CHECK-NEXT: - { srcinst: [[DBG_INSTR_NUM3:[0-9+]]], srcop: [[DBG_INSTR_OP2:[0-9+]]], dstinst: [[DBG_INSTR_NUM4:[0-9+]]], dstop: 0, subreg: 0 }
13+
14+
# CHECK: $w[[REG1:[0-9+]]], renamable $w[[REG2:[0-9+]]] = LDPWi renamable $x[[REG1]], 0, debug-instr-number [[DBG_INSTR_NUM2]]
15+
# CHECK-NEXT: $w[[REG1]] = KILL $w[[REG1]], implicit-def $x[[REG1]]
16+
# CHECK-NEXT: $x[[REG1]] = SBFMXri $x[[REG1]], 0, 31, debug-instr-number [[DBG_INSTR_NUM4]]
17+
# 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+]}}
18+
# 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+]}}
19+
20+
# 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.
21+
22+
# CHECK-LABEL: name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE2
23+
# CHECK: debugValueSubstitutions:
24+
# CHECK-NEXT: - { srcinst: [[DBG_INSTR_NUM5:[0-9+]]], srcop: [[DBG_INSTR_OP3:[0-9+]]], dstinst: [[DBG_INSTR_NUM6:[0-9+]]], dstop: 1, subreg: 0 }
25+
# CHECK-NEXT: - { srcinst: [[DBG_INSTR_NUM7:[0-9+]]], srcop: [[DBG_INSTR_OP4:[0-9+]]], dstinst: [[DBG_INSTR_NUM6]], dstop: 0, subreg: 0 }
26+
27+
# CHECK: renamable $x[[REG3:[0-9+]]], renamable $x[[REG4:[0-9+]]] = LDPXi renamable $x[[REG3]], 0, debug-instr-number [[DBG_INSTR_NUM6]]
28+
# 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
29+
# 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
30+
31+
--- |
32+
define i64 @_ZNK4llvm9StringRef4sizeEv(ptr readonly captures(none) %this) local_unnamed_addr #0 {
33+
entry:
34+
%Length = getelementptr i8, ptr %this, i64 8
35+
%0 = load i64, ptr %Length, align 4
36+
ret i64 %0
37+
}
38+
define ptr @_ZNK4llvm9StringRef4dataEv(ptr readonly captures(none) %this) local_unnamed_addr #0 {
39+
entry:
40+
%0 = load ptr, ptr %this, align 8
41+
ret ptr %0
42+
}
43+
define void @_ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE(ptr readonly captures(none) %agg.result) local_unnamed_addr !dbg !3 {
44+
%call1541 = load volatile ptr, ptr null, align 4294967296, !dbg !9
45+
%FullMatch.sroa.1.0.agg.result.sroa_idx = getelementptr inbounds nuw i8, ptr %agg.result, i64 8
46+
ret void
47+
}
48+
define void @_ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE2(ptr readonly captures(none) %agg.result) local_unnamed_addr !dbg !10 {
49+
%call1541 = load volatile ptr, ptr null, align 4294967296, !dbg !11
50+
%FullMatch.sroa.1.0.agg.result.sroa_idx = getelementptr inbounds nuw i8, ptr %agg.result, i64 8
51+
ret void
52+
}
53+
!llvm.module.flags = !{!0}
54+
!llvm.dbg.cu = !{!1}
55+
!0 = !{i32 2, !"Debug Info Version", i32 3}
56+
!1 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !2, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, sdk: "MacOSX15.3.sdk")
57+
!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")
58+
!3 = distinct !DISubprogram(scope: null, type: !4, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !6)
59+
!4 = distinct !DISubroutineType(types: !5)
60+
!5 = !{}
61+
!6 = !{!7}
62+
!7 = !DILocalVariable(name: "FullMatch", scope: !3, line: 1152, type: !8)
63+
!8 = distinct !DICompositeType(tag: DW_TAG_class_type, size: 128, identifier: "_ZTSN4llvm9StringRefE")
64+
!9 = !DILocation(line: 0, scope: !3)
65+
!10 = distinct !DISubprogram(scope: null, type: !4, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !12)
66+
!11 = !DILocation(line: 0, scope: !10)
67+
!12 = !{!13}
68+
!13 = !DILocalVariable(name: "FullMatch", scope: !10, line: 1152, type: !14)
69+
!14 = distinct !DICompositeType(tag: DW_TAG_class_type, size: 128, identifier: "_ZTSN4llvm9StringRefE")
70+
71+
name: _ZNK4llvm9StringRef4sizeEv
72+
---
73+
name: _ZNK4llvm9StringRef4dataEv
74+
...
75+
name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE
76+
debugValueSubstitutions: []
77+
body: |
78+
bb.0 (%ir-block.0):
79+
renamable $w1 = LDRWui renamable $x0, 1, debug-instr-number 1 :: (load (s64) from %ir.FullMatch.sroa.1.0.agg.result.sroa_idx, align 1)
80+
DBG_INSTR_REF !7, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref(1, 0), debug-location !9
81+
renamable $x0 = LDRSWui killed renamable $x0, 0, debug-instr-number 2 :: (load (s64) from %ir.agg.result)
82+
DBG_INSTR_REF !7, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref(2, 0), debug-location !9
83+
...
84+
name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE2
85+
debugValueSubstitutions: []
86+
body: |
87+
bb.0 (%ir-block.0):
88+
renamable $x1 = LDRXui renamable $x0, 1, debug-instr-number 1 :: (load (s64) from %ir.FullMatch.sroa.1.0.agg.result.sroa_idx, align 1)
89+
DBG_INSTR_REF !13, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 64), dbg-instr-ref(1, 0), debug-location !11
90+
renamable $x0 = LDRXui killed renamable $x0, 0, debug-instr-number 2 :: (load (s64) from %ir.agg.result)
91+
DBG_INSTR_REF !13, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref(2, 0), debug-location !11

0 commit comments

Comments
 (0)