Skip to content

Commit 66e78da

Browse files
rastogishubhamrastogishubham
committed
Merge sourcelocation in CSEMIRBuilder::getDominatingInstrForID.
Make sure to merge the sourcelocation of the Dominating Instruction that is hoisted in a basic block in the CSEMIRBuilder in the legalizer pass. If this is not done, we can have a incorrect line table entry that makes the instruction pointer jump around. For example the line table without this patch looks like: Address Line Column File ISA Discriminator OpIndex Flags ------------------ ------ ------ ------ --- ------------- ------- ------------- 0x0000000000000000 0 0 1 0 0 0 is_stmt 0x0000000000000010 11 14 1 0 0 0 is_stmt prologue_end 0x0000000000000028 12 1 1 0 0 0 is_stmt 0x000000000000002c 12 15 1 0 0 0 0x000000000000004c 12 13 1 0 0 0 0x000000000000005c 13 1 1 0 0 0 is_stmt 0x0000000000000064 12 13 1 0 0 0 is_stmt 0x000000000000007c 13 7 1 0 0 0 is_stmt 0x00000000000000c8 13 1 1 0 0 0 0x00000000000000e8 13 1 1 0 0 0 epilogue_begin 0x00000000000000f8 13 1 1 0 0 0 end_sequence The line table entry for 0x000000000000005c should be 0
1 parent 37277d8 commit 66e78da

File tree

6 files changed

+93
-17
lines changed

6 files changed

+93
-17
lines changed

‎llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ class CSEMIRBuilder : public MachineIRBuilder {
4747
/// dominates the current insertion point and if not, move it just before the
4848
/// current insertion point and return it. If not found, return Null
4949
/// MachineInstrBuilder.
50-
MachineInstrBuilder getDominatingInstrForID(FoldingSetNodeID &ID,
51-
void *&NodeInsertPos);
50+
MachineInstrBuilder
51+
getDominatingInstrForID(FoldingSetNodeID &ID, void *&NodeInsertPos,
52+
SmallVectorImpl<DILocation *> *Locs = nullptr);
5253
/// Simple check if we can CSE (we have the CSEInfo) or if this Opcode is
5354
/// safe to CSE.
5455
bool canPerformCSEForOpc(unsigned Opc) const;
@@ -97,8 +98,9 @@ class CSEMIRBuilder : public MachineIRBuilder {
9798
// Bring in the other overload from the base class.
9899
using MachineIRBuilder::buildConstant;
99100

100-
MachineInstrBuilder buildConstant(const DstOp &Res,
101-
const ConstantInt &Val) override;
101+
MachineInstrBuilder
102+
buildConstant(const DstOp &Res, const ConstantInt &Val,
103+
SmallVectorImpl<DILocation *> *Locs = nullptr) override;
102104

103105
// Bring in the other overload from the base class.
104106
using MachineIRBuilder::buildFConstant;

‎llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,12 @@ class LegalizationArtifactCombiner {
9999
const LLT DstTy = MRI.getType(DstReg);
100100
if (isInstLegal({TargetOpcode::G_CONSTANT, {DstTy}})) {
101101
auto &CstVal = SrcMI->getOperand(1);
102+
SmallVector<DILocation *> Locs;
103+
Locs.push_back(MI.getDebugLoc().get());
104+
Locs.push_back(SrcMI->getDebugLoc().get());
102105
Builder.buildConstant(
103-
DstReg, CstVal.getCImm()->getValue().sext(DstTy.getSizeInBits()));
106+
DstReg, CstVal.getCImm()->getValue().sext(DstTy.getSizeInBits()),
107+
&Locs);
104108
UpdatedDefs.push_back(DstReg);
105109
markInstAndDefDead(MI, *SrcMI, DeadInsts);
106110
return true;

‎llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -856,8 +856,9 @@ class MachineIRBuilder {
856856
/// type.
857857
///
858858
/// \return The newly created instruction.
859-
virtual MachineInstrBuilder buildConstant(const DstOp &Res,
860-
const ConstantInt &Val);
859+
virtual MachineInstrBuilder
860+
buildConstant(const DstOp &Res, const ConstantInt &Val,
861+
SmallVectorImpl<DILocation *> *Locs = nullptr);
861862

862863
/// Build and insert \p Res = G_CONSTANT \p Val
863864
///
@@ -868,7 +869,9 @@ class MachineIRBuilder {
868869
///
869870
/// \return The newly created instruction.
870871
MachineInstrBuilder buildConstant(const DstOp &Res, int64_t Val);
871-
MachineInstrBuilder buildConstant(const DstOp &Res, const APInt &Val);
872+
MachineInstrBuilder
873+
buildConstant(const DstOp &Res, const APInt &Val,
874+
SmallVectorImpl<DILocation *> *Locs = nullptr);
872875

873876
/// Build and insert \p Res = G_FCONSTANT \p Val
874877
///

‎llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ bool CSEMIRBuilder::dominates(MachineBasicBlock::const_iterator A,
3636

3737
MachineInstrBuilder
3838
CSEMIRBuilder::getDominatingInstrForID(FoldingSetNodeID &ID,
39-
void *&NodeInsertPos) {
39+
void *&NodeInsertPos,
40+
SmallVectorImpl<DILocation *> *Locs) {
4041
GISelCSEInfo *CSEInfo = getCSEInfo();
4142
assert(CSEInfo && "Can't get here without setting CSEInfo");
4243
MachineBasicBlock *CurMBB = &getMBB();
@@ -51,6 +52,11 @@ CSEMIRBuilder::getDominatingInstrForID(FoldingSetNodeID &ID,
5152
// this builder will have the def ready.
5253
setInsertPt(*CurMBB, std::next(MII));
5354
} else if (!dominates(MI, CurrPos)) {
55+
if (Locs) {
56+
Locs->push_back(MI->getDebugLoc().get());
57+
auto *Loc = DILocation::getMergedLocations(*Locs);
58+
MI->setDebugLoc(Loc);
59+
}
5460
CurMBB->splice(CurrPos, CurMBB, MI);
5561
}
5662
return MachineInstrBuilder(getMF(), MI);
@@ -320,8 +326,9 @@ MachineInstrBuilder CSEMIRBuilder::buildInstr(unsigned Opc,
320326
return memoizeMI(NewMIB, InsertPos);
321327
}
322328

323-
MachineInstrBuilder CSEMIRBuilder::buildConstant(const DstOp &Res,
324-
const ConstantInt &Val) {
329+
MachineInstrBuilder
330+
CSEMIRBuilder::buildConstant(const DstOp &Res, const ConstantInt &Val,
331+
SmallVectorImpl<DILocation *> *Locs) {
325332
constexpr unsigned Opc = TargetOpcode::G_CONSTANT;
326333
if (!canPerformCSEForOpc(Opc))
327334
return MachineIRBuilder::buildConstant(Res, Val);
@@ -337,7 +344,7 @@ MachineInstrBuilder CSEMIRBuilder::buildConstant(const DstOp &Res,
337344
profileMBBOpcode(ProfBuilder, Opc);
338345
profileDstOp(Res, ProfBuilder);
339346
ProfBuilder.addNodeIDMachineOperand(MachineOperand::CreateCImm(&Val));
340-
MachineInstrBuilder MIB = getDominatingInstrForID(ID, InsertPos);
347+
MachineInstrBuilder MIB = getDominatingInstrForID(ID, InsertPos, Locs);
341348
if (MIB) {
342349
// Handle generating copies here.
343350
return generateCopiesIfRequired({Res}, MIB);

‎llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,9 @@ MachineInstrBuilder MachineIRBuilder::buildCopy(const DstOp &Res,
314314
return buildInstr(TargetOpcode::COPY, Res, Op);
315315
}
316316

317-
MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
318-
const ConstantInt &Val) {
317+
MachineInstrBuilder
318+
MachineIRBuilder::buildConstant(const DstOp &Res, const ConstantInt &Val,
319+
SmallVectorImpl<DILocation *> *Locs) {
319320
LLT Ty = Res.getLLTTy(*getMRI());
320321
LLT EltTy = Ty.getScalarType();
321322
assert(EltTy.getScalarSizeInBits() == Val.getBitWidth() &&
@@ -375,10 +376,11 @@ MachineInstrBuilder MachineIRBuilder::buildFConstant(const DstOp &Res,
375376
return Const;
376377
}
377378

378-
MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
379-
const APInt &Val) {
379+
MachineInstrBuilder
380+
MachineIRBuilder::buildConstant(const DstOp &Res, const APInt &Val,
381+
SmallVectorImpl<DILocation *> *Locs) {
380382
ConstantInt *CI = ConstantInt::get(getMF().getFunction().getContext(), Val);
381-
return buildConstant(Res, *CI);
383+
return buildConstant(Res, *CI, Locs);
382384
}
383385

384386
MachineInstrBuilder MachineIRBuilder::buildFConstant(const DstOp &Res,

‎llvm/test/DebugInfo/merge-locations-legalizer.mir

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# RUN: llc %s -O0 --start-before=legalizer --stop-after=legalizer -o - | FileCheck %s
2+
# CHECK-NOT: %35:_(s32) = G_CONSTANT i32 0, debug-location !71
3+
# CHECK: %35:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 0,
4+
--- |
5+
define i32 @main(i32 %0, ptr %1) #0 !dbg !57 {
6+
entry:
7+
ret i32 0, !dbg !71
8+
}
9+
!3 = !DIFile(filename: "/Volumes/Data/swift/llvm-project/lldb/test/API/lang/swift/external_provider_extra_inhabitants/main.swift", directory: "/Volumes/Data/swift")
10+
!23 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !3, sdk: "MacOSX14.2.sdk")
11+
!57 = distinct !DISubprogram(name: "main", unit: !23)
12+
!64 = distinct !DILexicalBlock(scope: !57, column: 1)
13+
!66 = distinct !DILexicalBlock(scope: !64, column: 1)
14+
!68 = !DILocation(line: 12, scope: !66)
15+
!70 = distinct !DILexicalBlock(scope: !66, column: 1)
16+
!71 = !DILocation(line: 13, scope: !70)
17+
name: main
18+
registers:
19+
- { id: 0, class: _, preferred-register: '' }
20+
- { id: 1, class: _, preferred-register: '' }
21+
- { id: 2, class: _, preferred-register: '' }
22+
- { id: 3, class: _, preferred-register: '' }
23+
- { id: 4, class: _, preferred-register: '' }
24+
- { id: 5, class: _, preferred-register: '' }
25+
- { id: 6, class: _, preferred-register: '' }
26+
- { id: 7, class: _, preferred-register: '' }
27+
- { id: 8, class: _, preferred-register: '' }
28+
- { id: 9, class: _, preferred-register: '' }
29+
- { id: 10, class: _, preferred-register: '' }
30+
- { id: 11, class: _, preferred-register: '' }
31+
- { id: 12, class: _, preferred-register: '' }
32+
- { id: 13, class: _, preferred-register: '' }
33+
- { id: 14, class: _, preferred-register: '' }
34+
- { id: 15, class: gpr64, preferred-register: '' }
35+
- { id: 18, class: _, preferred-register: '' }
36+
- { id: 19, class: _, preferred-register: '' }
37+
- { id: 20, class: _, preferred-register: '' }
38+
- { id: 21, class: _, preferred-register: '' }
39+
- { id: 22, class: _, preferred-register: '' }
40+
- { id: 23, class: _, preferred-register: '' }
41+
- { id: 24, class: _, preferred-register: '' }
42+
- { id: 25, class: _, preferred-register: '' }
43+
- { id: 26, class: _, preferred-register: '' }
44+
- { id: 27, class: _, preferred-register: '' }
45+
- { id: 28, class: _, preferred-register: '' }
46+
- { id: 29, class: _, preferred-register: '' }
47+
- { id: 30, class: _, preferred-register: '' }
48+
- { id: 31, class: _, preferred-register: '' }
49+
- { id: 32, class: _, preferred-register: '' }
50+
- { id: 33, class: _, preferred-register: '' }
51+
- { id: 34, class: _, preferred-register: '' }
52+
body: |
53+
bb.1.entry:
54+
%16:_(s8) = G_CONSTANT i8 0, debug-location !68
55+
%17:_(s32) = G_ANYEXT %16(s8), debug-location !68
56+
$w2 = COPY %17(s32), debug-location !68
57+
%35:_(s32) = G_CONSTANT i32 0, debug-location !71
58+
$w0 = COPY %35(s32), debug-location !71

0 commit comments

Comments
 (0)