Skip to content

Commit 88d90e6

Browse files
rastogishubhamrastogishubham
committed
addressed review comments
1 parent 75664e4 commit 88d90e6

File tree

2 files changed

+139
-73
lines changed

2 files changed

+139
-73
lines changed

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

Lines changed: 120 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -965,38 +965,26 @@ static void updateDefinedRegisters(MachineInstr &MI, LiveRegUnits &Units,
965965
Units.addReg(MOP.getReg());
966966
}
967967

968-
/// Find the DBG_INSTR_REF instruction that references the \p InstrNum
969-
static std::optional<MachineInstr *> findDebugInstrRef(MachineBasicBlock *MBB,
970-
unsigned InstrNum) {
971-
972-
for (auto &MI : *MBB) {
973-
if (MI.isDebugRef())
974-
if (MI.getOperand(2).getInstrRefInstrIndex() == InstrNum)
975-
return &MI;
968+
/// This function will add a new entry into the debugValueSubstitutions table
969+
/// when two instruction have been merged into a new one represented by \p
970+
/// MergedInstr.
971+
static void addDebugSubstitutionsToTable(MachineFunction *MF,
972+
unsigned InstrNumToSet,
973+
MachineInstr &OriginalInstr,
974+
MachineInstr &MergedInstr) {
975+
976+
// Figure out the Operand Index of the destination register of the
977+
// OriginalInstr in the new MergedInstr.
978+
auto Reg = OriginalInstr.getOperand(0).getReg();
979+
unsigned OperandNo = 0;
980+
for (auto Op : MergedInstr.operands()) {
981+
if (Op.getReg() == Reg)
982+
break;
983+
OperandNo++;
976984
}
977-
return std::nullopt;
978-
}
979985

980-
/// Set the correct debug-instr-number and the operand index in case of a merge.
981-
static void setDebugInstrNum(MachineBasicBlock::iterator OriginalInstr,
982-
MachineInstrBuilder &MIB, unsigned InstrNumToFind,
983-
unsigned InstrNumToSet, MachineBasicBlock *MBB) {
984-
985-
auto InstrRefMI = findDebugInstrRef(MBB, InstrNumToFind);
986-
if (InstrRefMI) {
987-
auto *MI = *InstrRefMI;
988-
MI->getOperand(2).setInstrRefInstrIndex(InstrNumToSet);
989-
// Set the Instruction Reference Op Index to be the same as the operand of
990-
// the new merged pair instruction.
991-
auto Reg = OriginalInstr->getOperand(0).getReg();
992-
unsigned OperandNo = 0;
993-
for (auto Op : MIB->operands()) {
994-
if (Op.getReg() == Reg)
995-
break;
996-
OperandNo++;
997-
}
998-
MI->getOperand(2).setInstrRefOpIndex(OperandNo);
999-
}
986+
MF->makeDebugValueSubstitution({OriginalInstr.peekDebugInstrNum(), 0},
987+
{InstrNumToSet, OperandNo});
1000988
}
1001989

1002990
MachineBasicBlock::iterator
@@ -1262,26 +1250,75 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
12621250
.addImm(31);
12631251
(void)MIBSXTW;
12641252

1253+
// In the case of a sign-extend, where we have something like:
1254+
// debugValueSubstitutions:[]
1255+
// $w1 = LDRWui $x0, 1, debug-instr-number 1
1256+
// DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
1257+
// $x0 = LDRSWui $x0, 0, debug-instr-number 2
1258+
// DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
1259+
1260+
// It will be converted to:
1261+
// debugValueSubstitutions:[]
1262+
// $w0, $w1 = LDPWi $x0, 0
1263+
// $w0 = KILL $w0, implicit-def $x0
1264+
// $x0 = SBFMXri $x0, 0, 31
1265+
// DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
1266+
// DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
1267+
1268+
// $x0 is where the final value is stored, so the sign extend (SBFMXri)
1269+
// instruction contains the final value we care about we give it a new
1270+
// debug-instr-number 3. Whereas, $w1 contains the final value that we care
1271+
// about, therefore the LDP instruction is also given a new
1272+
// debug-instr-number 4. We have to add these subsitutions to the
1273+
// debugValueSubstitutions table. However, we also have to ensure that the
1274+
// OpIndex that pointed to debug-instr-number 1 gets updated to 1, because
1275+
// $w1 is the second operand of the LDP instruction.
1276+
1277+
// We want the final result to look like:
1278+
// debugValueSubstitutions:
1279+
// - { srcinst: 1, srcop: 0, dstinst: 4, dstop: 1, subreg: 0 }
1280+
// - { srcinst: 2, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
1281+
// $w0, $w1 = LDPWi $x0, 0, debug-instr-number 4
1282+
// $w0 = KILL $w0, implicit-def $x0
1283+
// $x0 = SBFMXri $x0, 0, 31, debug-instr-number 3
1284+
// DBG_INSTR_REF !7, dbg-instr-ref(1, 0), debug-location !9
1285+
// DBG_INSTR_REF !8, dbg-instr-ref(2, 0), debug-location !9
1286+
12651287
if (I->peekDebugInstrNum()) {
1266-
auto InstrNum = I->peekDebugInstrNum();
1267-
// If I is the instruction which got sign extended, restore the debug
1268-
// instruction number from I to the SBFMXri instruction
1269-
if (DstRegX == I->getOperand(0).getReg())
1270-
MIBSXTW->setDebugInstrNum(InstrNum);
1271-
else {
1272-
MIB->setDebugInstrNum(InstrNum);
1273-
setDebugInstrNum(I, MIB, InstrNum, InstrNum, MBB);
1288+
// If I is the instruction which got sign extended and has a
1289+
// debug-instr-number, give the SBFMXri instruction a new
1290+
// debug-instr-number, and update the debugValueSubstitutions table with
1291+
// the new debug-instr-number and OpIndex pair. Otherwise, give the Merged
1292+
// instruction a new debug-instr-number, and update the
1293+
// debugValueSubstitutions table with the new debug-instr-number and
1294+
// OpIndex pair.
1295+
unsigned NewInstrNum;
1296+
if (DstRegX == I->getOperand(0).getReg()) {
1297+
NewInstrNum = MIBSXTW->getDebugInstrNum();
1298+
addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *I,
1299+
*MIBSXTW);
1300+
} else {
1301+
NewInstrNum = MIB->getDebugInstrNum();
1302+
addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *I, *MIB);
12741303
}
12751304
}
12761305
if (Paired->peekDebugInstrNum()) {
1277-
auto InstrNum = Paired->peekDebugInstrNum();
1278-
// If Paired is the instruction which got sign extended, restore the debug
1279-
// instruction number from Paired to the SBFMXri instruction
1280-
if (DstRegX == Paired->getOperand(0).getReg())
1281-
MIBSXTW->setDebugInstrNum(InstrNum);
1282-
else {
1283-
MIB->setDebugInstrNum(InstrNum);
1284-
setDebugInstrNum(Paired, MIB, InstrNum, InstrNum, MBB);
1306+
// If Paired is the instruction which got sign extended and has a
1307+
// debug-instr-number, give the SBFMXri instruction a new
1308+
// debug-instr-number, and update the debugValueSubstitutions table with
1309+
// the new debug-instr-number and OpIndex pair. Otherwise, give the Merged
1310+
// instruction a new debug-instr-number, and update the
1311+
// debugValueSubstitutions table with the new debug-instr-number and
1312+
// OpIndex pair.
1313+
unsigned NewInstrNum;
1314+
if (DstRegX == Paired->getOperand(0).getReg()) {
1315+
NewInstrNum = MIBSXTW->getDebugInstrNum();
1316+
addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *Paired,
1317+
*MIBSXTW);
1318+
} else {
1319+
NewInstrNum = MIB->getDebugInstrNum();
1320+
addDebugSubstitutionsToTable(MBB->getParent(), NewInstrNum, *Paired,
1321+
*MIB);
12851322
}
12861323
}
12871324

@@ -1299,27 +1336,44 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
12991336
LLVM_DEBUG(((MachineInstr *)MIB)->print(dbgs()));
13001337
} else {
13011338

1302-
// Restore the debug instruction numbers to the merged instruction.
1303-
bool UseNewDebugInstrNum = false;
1304-
unsigned NewDebugInstrNum;
1305-
if (I->peekDebugInstrNum() && Paired->peekDebugInstrNum()) {
1306-
// Both instructions contain debug instruction numbers. We need to set a
1307-
// new instruction number for the paired instruction and update the
1308-
// DBG_INSTR_REFs that reference the instruction numbers of both
1309-
// instructions and update them.
1310-
NewDebugInstrNum = MIB->getDebugInstrNum();
1311-
UseNewDebugInstrNum = true;
1339+
// In the case that the merge doesn't result in a sign-extend, if we have
1340+
// something like:
1341+
// debugValueSubstitutions:[]
1342+
// $x1 = LDRXui $x0, 1, debug-instr-number 1
1343+
// DBG_INSTR_REF !13, dbg-instr-ref(1, 0), debug-location !11
1344+
// $x0 = LDRXui killed $x0, 0, debug-instr-number 2
1345+
// DBG_INSTR_REF !14, dbg-instr-ref(2, 0), debug-location !11
1346+
1347+
// It will be converted to:
1348+
// debugValueSubstitutions: []
1349+
// $x0, $x1 = LDPXi $x0, 0
1350+
// DBG_INSTR_REF !12, dbg-instr-ref(1, 0), debug-location !14
1351+
// DBG_INSTR_REF !13, dbg-instr-ref(2, 0), debug-location !14
1352+
1353+
// Here all that needs to be done is, that the LDP instruction needs to be
1354+
// updated with a new debug-instr-number, we then need to add entries into
1355+
// the debugSubstitutions table to map the old instr-refs to the new ones.
1356+
1357+
// We want the final result to look like:
1358+
// debugValueSubstitutions:
1359+
// - { srcinst: 1, srcop: 0, dstinst: 3, dstop: 1, subreg: 0 }
1360+
// - { srcinst: 2, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
1361+
// $x0, $x1 = LDPXi $x0, 0, debug-instr-number 3
1362+
// DBG_INSTR_REF !12, dbg-instr-ref(1, 0), debug-location !14
1363+
// DBG_INSTR_REF !12, dbg-instr-ref(2, 0), debug-location !14
1364+
1365+
// Assign new DebugInstrNum to the Paired instruction.
1366+
1367+
if (I->peekDebugInstrNum()) {
1368+
unsigned NewDebugInstrNum = MIB->getDebugInstrNum();
1369+
addDebugSubstitutionsToTable(MBB->getParent(), NewDebugInstrNum, *I,
1370+
*MIB);
1371+
}
1372+
if (Paired->peekDebugInstrNum()) {
1373+
unsigned NewDebugInstrNum = MIB->getDebugInstrNum();
1374+
addDebugSubstitutionsToTable(MBB->getParent(), NewDebugInstrNum, *Paired,
1375+
*MIB);
13121376
}
1313-
if (I->peekDebugInstrNum())
1314-
setDebugInstrNum(
1315-
I, MIB, I->peekDebugInstrNum(),
1316-
UseNewDebugInstrNum ? NewDebugInstrNum : I->peekDebugInstrNum(), MBB);
1317-
1318-
if (Paired->peekDebugInstrNum())
1319-
setDebugInstrNum(Paired, MIB, Paired->peekDebugInstrNum(),
1320-
UseNewDebugInstrNum ? NewDebugInstrNum
1321-
: Paired->peekDebugInstrNum(),
1322-
MBB);
13231377

13241378
LLVM_DEBUG(((MachineInstr *)MIB)->print(dbgs()));
13251379
}

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

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,27 @@
66

77
# 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.
88

9-
# CHECK: $w[[REG1:[0-9]+]], renamable $w[[REG2:[0-9]+]] = LDPWi renamable ${{[a-z0-9]+}}, 0, debug-instr-number [[DBG_INSTR_NUM1:[0-9]+]]
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]]
1015
# CHECK-NEXT: $w[[REG1]] = KILL $w[[REG1]], implicit-def $x[[REG1]]
11-
# CHECK-NEXT: $x[[REG1]] = SBFMXri $x[[REG1]], 0, 31, debug-instr-number [[DBG_INSTR_NUM2:[0-9]+]]
12-
# 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]+]]
13-
# 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]]
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+]}}
1419

1520
# 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.
1621

17-
# 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]+]]
18-
# 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]+]]
19-
# 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]]
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
2030

2131
--- |
2232
define i64 @_ZNK4llvm9StringRef4sizeEv(ptr readonly captures(none) %this) local_unnamed_addr #0 {
@@ -63,6 +73,7 @@ name: _ZNK4llvm9StringRef4sizeEv
6373
name: _ZNK4llvm9StringRef4dataEv
6474
...
6575
name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE
76+
debugValueSubstitutions: []
6677
body: |
6778
bb.0 (%ir-block.0):
6879
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: |
7182
DBG_INSTR_REF !7, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 64, 32), dbg-instr-ref(2, 0), debug-location !9
7283
...
7384
name: _ZNK4llvm7Pattern5matchENS_9StringRefERKNS_9SourceMgrE2
85+
debugValueSubstitutions: []
7486
body: |
7587
bb.0 (%ir-block.0):
7688
renamable $x1 = LDRXui renamable $x0, 1, debug-instr-number 1 :: (load (s64) from %ir.FullMatch.sroa.1.0.agg.result.sroa_idx, align 1)

0 commit comments

Comments
 (0)