Skip to content

Commit c12c682

Browse files
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 c12c682

File tree

6 files changed

+67
-14
lines changed

6 files changed

+67
-14
lines changed

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ class CSEMIRBuilder : public MachineIRBuilder {
4848
/// current insertion point and return it. If not found, return Null
4949
/// MachineInstrBuilder.
5050
MachineInstrBuilder getDominatingInstrForID(FoldingSetNodeID &ID,
51-
void *&NodeInsertPos);
51+
void *&NodeInsertPos,
52+
DILocation *Location = 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,8 @@ 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 buildConstant(const DstOp &Res, const ConstantInt &Val,
102+
DILocation *Location = nullptr) override;
102103

103104
// Bring in the other overload from the base class.
104105
using MachineIRBuilder::buildFConstant;

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "llvm/CodeGen/Register.h"
2727
#include "llvm/CodeGen/TargetOpcodes.h"
2828
#include "llvm/IR/Constants.h"
29+
#include "llvm/IR/DebugInfoMetadata.h"
2930
#include "llvm/Support/Debug.h"
3031

3132
#define DEBUG_TYPE "legalizer"
@@ -99,8 +100,11 @@ class LegalizationArtifactCombiner {
99100
const LLT DstTy = MRI.getType(DstReg);
100101
if (isInstLegal({TargetOpcode::G_CONSTANT, {DstTy}})) {
101102
auto &CstVal = SrcMI->getOperand(1);
103+
auto *MergedLocation = DILocation::getMergedLocation(
104+
MI.getDebugLoc().get(), SrcMI->getDebugLoc().get());
102105
Builder.buildConstant(
103-
DstReg, CstVal.getCImm()->getValue().sext(DstTy.getSizeInBits()));
106+
DstReg, CstVal.getCImm()->getValue().sext(DstTy.getSizeInBits()),
107+
MergedLocation);
104108
UpdatedDefs.push_back(DstReg);
105109
markInstAndDefDead(MI, *SrcMI, DeadInsts);
106110
return true;

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,10 @@ class MachineIRBuilder {
243243
}
244244

245245
public:
246+
/// This is the merged location of an instruction which is a result of a fold
247+
/// in the legalizer.
248+
DILocation *Location = nullptr;
249+
246250
/// Some constructors for easy use.
247251
MachineIRBuilder() = default;
248252
MachineIRBuilder(MachineFunction &MF) { setMF(MF); }
@@ -857,7 +861,8 @@ class MachineIRBuilder {
857861
///
858862
/// \return The newly created instruction.
859863
virtual MachineInstrBuilder buildConstant(const DstOp &Res,
860-
const ConstantInt &Val);
864+
const ConstantInt &Val,
865+
DILocation *Location = nullptr);
861866

862867
/// Build and insert \p Res = G_CONSTANT \p Val
863868
///
@@ -868,7 +873,8 @@ class MachineIRBuilder {
868873
///
869874
/// \return The newly created instruction.
870875
MachineInstrBuilder buildConstant(const DstOp &Res, int64_t Val);
871-
MachineInstrBuilder buildConstant(const DstOp &Res, const APInt &Val);
876+
MachineInstrBuilder buildConstant(const DstOp &Res, const APInt &Val,
877+
DILocation *Location = nullptr);
872878

873879
/// Build and insert \p Res = G_FCONSTANT \p Val
874880
///

llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,8 @@ bool CSEMIRBuilder::dominates(MachineBasicBlock::const_iterator A,
3434
return &*I == A;
3535
}
3636

37-
MachineInstrBuilder
38-
CSEMIRBuilder::getDominatingInstrForID(FoldingSetNodeID &ID,
39-
void *&NodeInsertPos) {
37+
MachineInstrBuilder CSEMIRBuilder::getDominatingInstrForID(
38+
FoldingSetNodeID &ID, void *&NodeInsertPos, DILocation *Location) {
4039
GISelCSEInfo *CSEInfo = getCSEInfo();
4140
assert(CSEInfo && "Can't get here without setting CSEInfo");
4241
MachineBasicBlock *CurMBB = &getMBB();
@@ -51,6 +50,16 @@ CSEMIRBuilder::getDominatingInstrForID(FoldingSetNodeID &ID,
5150
// this builder will have the def ready.
5251
setInsertPt(*CurMBB, std::next(MII));
5352
} else if (!dominates(MI, CurrPos)) {
53+
// Location represents the DILocation of an instruction (or merged
54+
// location of instructions, if multiple instructions were folded) at the
55+
// insertion point where MI is going to be moved to, since MI is going to
56+
// be moved, it's DILocation needs to be updated to make sure the line
57+
// table is correct.
58+
if (Location) {
59+
auto *Loc =
60+
DILocation::getMergedLocation(Location, MI->getDebugLoc().get());
61+
MI->setDebugLoc(Loc);
62+
}
5463
CurMBB->splice(CurrPos, CurMBB, MI);
5564
}
5665
return MachineInstrBuilder(getMF(), MI);
@@ -321,7 +330,8 @@ MachineInstrBuilder CSEMIRBuilder::buildInstr(unsigned Opc,
321330
}
322331

323332
MachineInstrBuilder CSEMIRBuilder::buildConstant(const DstOp &Res,
324-
const ConstantInt &Val) {
333+
const ConstantInt &Val,
334+
DILocation *Location) {
325335
constexpr unsigned Opc = TargetOpcode::G_CONSTANT;
326336
if (!canPerformCSEForOpc(Opc))
327337
return MachineIRBuilder::buildConstant(Res, Val);
@@ -337,7 +347,7 @@ MachineInstrBuilder CSEMIRBuilder::buildConstant(const DstOp &Res,
337347
profileMBBOpcode(ProfBuilder, Opc);
338348
profileDstOp(Res, ProfBuilder);
339349
ProfBuilder.addNodeIDMachineOperand(MachineOperand::CreateCImm(&Val));
340-
MachineInstrBuilder MIB = getDominatingInstrForID(ID, InsertPos);
350+
MachineInstrBuilder MIB = getDominatingInstrForID(ID, InsertPos, Location);
341351
if (MIB) {
342352
// Handle generating copies here.
343353
return generateCopiesIfRequired({Res}, MIB);

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,8 @@ MachineInstrBuilder MachineIRBuilder::buildCopy(const DstOp &Res,
315315
}
316316

317317
MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
318-
const ConstantInt &Val) {
318+
const ConstantInt &Val,
319+
DILocation *Location) {
319320
LLT Ty = Res.getLLTTy(*getMRI());
320321
LLT EltTy = Ty.getScalarType();
321322
assert(EltTy.getScalarSizeInBits() == Val.getBitWidth() &&
@@ -376,9 +377,10 @@ MachineInstrBuilder MachineIRBuilder::buildFConstant(const DstOp &Res,
376377
}
377378

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

384386
MachineInstrBuilder MachineIRBuilder::buildFConstant(const DstOp &Res,
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# This test checks to make sure that when an instruction (%3 in the test) is
2+
# moved due to matching a result of a fold of two other instructions
3+
# (%1, and %2 in the test) in the legalizer, the DILocation of the
4+
# instruction that is moved (%3) is updated appropriately.
5+
6+
# RUN: llc %s -run-pass=legalizer -o - | FileCheck %s
7+
# CHECK-NOT: %2:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 13
8+
# CHECK: %2:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 0,
9+
--- |
10+
11+
define i32 @main(i32 %0, ptr %1) #0 !dbg !57 {
12+
entry:
13+
ret i32 0, !dbg !71
14+
}
15+
!3 = !DIFile(filename: "main.swift", directory: "/Volumes/Data/swift")
16+
!23 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !3, sdk: "blah.sdk")
17+
!57 = distinct !DISubprogram(name: "main", unit: !23)
18+
!64 = distinct !DILexicalBlock(scope: !57, column: 1)
19+
!66 = distinct !DILexicalBlock(scope: !64, column: 1)
20+
!68 = !DILocation(line: 12, scope: !66)
21+
!70 = distinct !DILexicalBlock(scope: !66, column: 1)
22+
!71 = !DILocation(line: 13, scope: !70)
23+
name: main
24+
body: |
25+
bb.0:
26+
%1:_(s8) = G_CONSTANT i8 0, debug-location !68
27+
%2:_(s32) = G_ANYEXT %1(s8), debug-location !68
28+
$w2 = COPY %2(s32), debug-location !68
29+
%3:_(s32) = G_CONSTANT i32 0, debug-location !71
30+
$w0 = COPY %3(s32), debug-location !71

0 commit comments

Comments
 (0)