[llvm] Merge sourcelocation in CSEMIRBuilder::getDominatingInstrForID. (PR #90922)
Shubham Sandeep Rastogi via llvm-commits
llvm-commits at lists.llvm.org
Thu May 2 17:21:10 PDT 2024
https://github.com/rastogishubham updated https://github.com/llvm/llvm-project/pull/90922
>From 66e78da665a2cf5814a6003693d441c2ed2da7e6 Mon Sep 17 00:00:00 2001
From: Shubham Sandeep Rastogi <srastogi22 at apple.com>
Date: Thu, 2 May 2024 15:17:12 -0700
Subject: [PATCH] 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
---
.../llvm/CodeGen/GlobalISel/CSEMIRBuilder.h | 10 ++--
.../GlobalISel/LegalizationArtifactCombiner.h | 6 +-
.../CodeGen/GlobalISel/MachineIRBuilder.h | 9 ++-
llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp | 15 +++--
.../CodeGen/GlobalISel/MachineIRBuilder.cpp | 12 ++--
.../DebugInfo/merge-locations-legalizer.mir | 58 +++++++++++++++++++
6 files changed, 93 insertions(+), 17 deletions(-)
create mode 100644 llvm/test/DebugInfo/merge-locations-legalizer.mir
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h b/llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h
index 488df12f13f8df..78341fa1b47bc9 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h
@@ -47,8 +47,9 @@ class CSEMIRBuilder : public MachineIRBuilder {
/// dominates the current insertion point and if not, move it just before the
/// current insertion point and return it. If not found, return Null
/// MachineInstrBuilder.
- MachineInstrBuilder getDominatingInstrForID(FoldingSetNodeID &ID,
- void *&NodeInsertPos);
+ MachineInstrBuilder
+ getDominatingInstrForID(FoldingSetNodeID &ID, void *&NodeInsertPos,
+ SmallVectorImpl<DILocation *> *Locs = nullptr);
/// Simple check if we can CSE (we have the CSEInfo) or if this Opcode is
/// safe to CSE.
bool canPerformCSEForOpc(unsigned Opc) const;
@@ -97,8 +98,9 @@ class CSEMIRBuilder : public MachineIRBuilder {
// Bring in the other overload from the base class.
using MachineIRBuilder::buildConstant;
- MachineInstrBuilder buildConstant(const DstOp &Res,
- const ConstantInt &Val) override;
+ MachineInstrBuilder
+ buildConstant(const DstOp &Res, const ConstantInt &Val,
+ SmallVectorImpl<DILocation *> *Locs = nullptr) override;
// Bring in the other overload from the base class.
using MachineIRBuilder::buildFConstant;
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
index 305bef7dd3ea63..266bdb80614f01 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
@@ -99,8 +99,12 @@ class LegalizationArtifactCombiner {
const LLT DstTy = MRI.getType(DstReg);
if (isInstLegal({TargetOpcode::G_CONSTANT, {DstTy}})) {
auto &CstVal = SrcMI->getOperand(1);
+ SmallVector<DILocation *> Locs;
+ Locs.push_back(MI.getDebugLoc().get());
+ Locs.push_back(SrcMI->getDebugLoc().get());
Builder.buildConstant(
- DstReg, CstVal.getCImm()->getValue().sext(DstTy.getSizeInBits()));
+ DstReg, CstVal.getCImm()->getValue().sext(DstTy.getSizeInBits()),
+ &Locs);
UpdatedDefs.push_back(DstReg);
markInstAndDefDead(MI, *SrcMI, DeadInsts);
return true;
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h b/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
index e15f7a7172e1a4..dc67a3321a0d9d 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
@@ -856,8 +856,9 @@ class MachineIRBuilder {
/// type.
///
/// \return The newly created instruction.
- virtual MachineInstrBuilder buildConstant(const DstOp &Res,
- const ConstantInt &Val);
+ virtual MachineInstrBuilder
+ buildConstant(const DstOp &Res, const ConstantInt &Val,
+ SmallVectorImpl<DILocation *> *Locs = nullptr);
/// Build and insert \p Res = G_CONSTANT \p Val
///
@@ -868,7 +869,9 @@ class MachineIRBuilder {
///
/// \return The newly created instruction.
MachineInstrBuilder buildConstant(const DstOp &Res, int64_t Val);
- MachineInstrBuilder buildConstant(const DstOp &Res, const APInt &Val);
+ MachineInstrBuilder
+ buildConstant(const DstOp &Res, const APInt &Val,
+ SmallVectorImpl<DILocation *> *Locs = nullptr);
/// Build and insert \p Res = G_FCONSTANT \p Val
///
diff --git a/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
index 551ba1e6036c17..893bebf4da327d 100644
--- a/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
@@ -36,7 +36,8 @@ bool CSEMIRBuilder::dominates(MachineBasicBlock::const_iterator A,
MachineInstrBuilder
CSEMIRBuilder::getDominatingInstrForID(FoldingSetNodeID &ID,
- void *&NodeInsertPos) {
+ void *&NodeInsertPos,
+ SmallVectorImpl<DILocation *> *Locs) {
GISelCSEInfo *CSEInfo = getCSEInfo();
assert(CSEInfo && "Can't get here without setting CSEInfo");
MachineBasicBlock *CurMBB = &getMBB();
@@ -51,6 +52,11 @@ CSEMIRBuilder::getDominatingInstrForID(FoldingSetNodeID &ID,
// this builder will have the def ready.
setInsertPt(*CurMBB, std::next(MII));
} else if (!dominates(MI, CurrPos)) {
+ if (Locs) {
+ Locs->push_back(MI->getDebugLoc().get());
+ auto *Loc = DILocation::getMergedLocations(*Locs);
+ MI->setDebugLoc(Loc);
+ }
CurMBB->splice(CurrPos, CurMBB, MI);
}
return MachineInstrBuilder(getMF(), MI);
@@ -320,8 +326,9 @@ MachineInstrBuilder CSEMIRBuilder::buildInstr(unsigned Opc,
return memoizeMI(NewMIB, InsertPos);
}
-MachineInstrBuilder CSEMIRBuilder::buildConstant(const DstOp &Res,
- const ConstantInt &Val) {
+MachineInstrBuilder
+CSEMIRBuilder::buildConstant(const DstOp &Res, const ConstantInt &Val,
+ SmallVectorImpl<DILocation *> *Locs) {
constexpr unsigned Opc = TargetOpcode::G_CONSTANT;
if (!canPerformCSEForOpc(Opc))
return MachineIRBuilder::buildConstant(Res, Val);
@@ -337,7 +344,7 @@ MachineInstrBuilder CSEMIRBuilder::buildConstant(const DstOp &Res,
profileMBBOpcode(ProfBuilder, Opc);
profileDstOp(Res, ProfBuilder);
ProfBuilder.addNodeIDMachineOperand(MachineOperand::CreateCImm(&Val));
- MachineInstrBuilder MIB = getDominatingInstrForID(ID, InsertPos);
+ MachineInstrBuilder MIB = getDominatingInstrForID(ID, InsertPos, Locs);
if (MIB) {
// Handle generating copies here.
return generateCopiesIfRequired({Res}, MIB);
diff --git a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
index 2e8407813ba641..5cbdfe6b531dff 100644
--- a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
@@ -314,8 +314,9 @@ MachineInstrBuilder MachineIRBuilder::buildCopy(const DstOp &Res,
return buildInstr(TargetOpcode::COPY, Res, Op);
}
-MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
- const ConstantInt &Val) {
+MachineInstrBuilder
+MachineIRBuilder::buildConstant(const DstOp &Res, const ConstantInt &Val,
+ SmallVectorImpl<DILocation *> *Locs) {
LLT Ty = Res.getLLTTy(*getMRI());
LLT EltTy = Ty.getScalarType();
assert(EltTy.getScalarSizeInBits() == Val.getBitWidth() &&
@@ -375,10 +376,11 @@ MachineInstrBuilder MachineIRBuilder::buildFConstant(const DstOp &Res,
return Const;
}
-MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
- const APInt &Val) {
+MachineInstrBuilder
+MachineIRBuilder::buildConstant(const DstOp &Res, const APInt &Val,
+ SmallVectorImpl<DILocation *> *Locs) {
ConstantInt *CI = ConstantInt::get(getMF().getFunction().getContext(), Val);
- return buildConstant(Res, *CI);
+ return buildConstant(Res, *CI, Locs);
}
MachineInstrBuilder MachineIRBuilder::buildFConstant(const DstOp &Res,
diff --git a/llvm/test/DebugInfo/merge-locations-legalizer.mir b/llvm/test/DebugInfo/merge-locations-legalizer.mir
new file mode 100644
index 00000000000000..92bd5de2858e69
--- /dev/null
+++ b/llvm/test/DebugInfo/merge-locations-legalizer.mir
@@ -0,0 +1,58 @@
+# RUN: llc %s -O0 --start-before=legalizer --stop-after=legalizer -o - | FileCheck %s
+# CHECK-NOT: %35:_(s32) = G_CONSTANT i32 0, debug-location !71
+# CHECK: %35:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 0,
+--- |
+ define i32 @main(i32 %0, ptr %1) #0 !dbg !57 {
+ entry:
+ ret i32 0, !dbg !71
+ }
+ !3 = !DIFile(filename: "/Volumes/Data/swift/llvm-project/lldb/test/API/lang/swift/external_provider_extra_inhabitants/main.swift", directory: "/Volumes/Data/swift")
+ !23 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !3, sdk: "MacOSX14.2.sdk")
+ !57 = distinct !DISubprogram(name: "main", unit: !23)
+ !64 = distinct !DILexicalBlock(scope: !57, column: 1)
+ !66 = distinct !DILexicalBlock(scope: !64, column: 1)
+ !68 = !DILocation(line: 12, scope: !66)
+ !70 = distinct !DILexicalBlock(scope: !66, column: 1)
+ !71 = !DILocation(line: 13, scope: !70)
+name: main
+registers:
+ - { id: 0, class: _, preferred-register: '' }
+ - { id: 1, class: _, preferred-register: '' }
+ - { id: 2, class: _, preferred-register: '' }
+ - { id: 3, class: _, preferred-register: '' }
+ - { id: 4, class: _, preferred-register: '' }
+ - { id: 5, class: _, preferred-register: '' }
+ - { id: 6, class: _, preferred-register: '' }
+ - { id: 7, class: _, preferred-register: '' }
+ - { id: 8, class: _, preferred-register: '' }
+ - { id: 9, class: _, preferred-register: '' }
+ - { id: 10, class: _, preferred-register: '' }
+ - { id: 11, class: _, preferred-register: '' }
+ - { id: 12, class: _, preferred-register: '' }
+ - { id: 13, class: _, preferred-register: '' }
+ - { id: 14, class: _, preferred-register: '' }
+ - { id: 15, class: gpr64, preferred-register: '' }
+ - { id: 18, class: _, preferred-register: '' }
+ - { id: 19, class: _, preferred-register: '' }
+ - { id: 20, class: _, preferred-register: '' }
+ - { id: 21, class: _, preferred-register: '' }
+ - { id: 22, class: _, preferred-register: '' }
+ - { id: 23, class: _, preferred-register: '' }
+ - { id: 24, class: _, preferred-register: '' }
+ - { id: 25, class: _, preferred-register: '' }
+ - { id: 26, class: _, preferred-register: '' }
+ - { id: 27, class: _, preferred-register: '' }
+ - { id: 28, class: _, preferred-register: '' }
+ - { id: 29, class: _, preferred-register: '' }
+ - { id: 30, class: _, preferred-register: '' }
+ - { id: 31, class: _, preferred-register: '' }
+ - { id: 32, class: _, preferred-register: '' }
+ - { id: 33, class: _, preferred-register: '' }
+ - { id: 34, class: _, preferred-register: '' }
+body: |
+ bb.1.entry:
+ %16:_(s8) = G_CONSTANT i8 0, debug-location !68
+ %17:_(s32) = G_ANYEXT %16(s8), debug-location !68
+ $w2 = COPY %17(s32), debug-location !68
+ %35:_(s32) = G_CONSTANT i32 0, debug-location !71
+ $w0 = COPY %35(s32), debug-location !71
More information about the llvm-commits
mailing list