Skip to content

Commit

Permalink
Merge sourcelocation in CSEMIRBuilder::getDominatingInstrForID. (#90922)
Browse files Browse the repository at this point in the history
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

After this patch, the line table 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      0      0      1   0             0       0 
0x0000000000000064     12     13      1   0             0       0 
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
```
  • Loading branch information
rastogishubham authored May 16, 2024
1 parent fa750f0 commit a9763de
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "llvm/CodeGen/Register.h"
#include "llvm/CodeGen/TargetOpcodes.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/Support/Debug.h"

#define DEBUG_TYPE "legalizer"
Expand Down Expand Up @@ -99,6 +100,11 @@ class LegalizationArtifactCombiner {
const LLT DstTy = MRI.getType(DstReg);
if (isInstLegal({TargetOpcode::G_CONSTANT, {DstTy}})) {
auto &CstVal = SrcMI->getOperand(1);
auto *MergedLocation = DILocation::getMergedLocation(
MI.getDebugLoc().get(), SrcMI->getDebugLoc().get());
// Set the debug location to the merged location of the SrcMI and the MI
// if the aext fold is successful.
Builder.setDebugLoc(MergedLocation);
Builder.buildConstant(
DstReg, CstVal.getCImm()->getValue().sext(DstTy.getSizeInBits()));
UpdatedDefs.push_back(DstReg);
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ CSEMIRBuilder::getDominatingInstrForID(FoldingSetNodeID &ID,
// this builder will have the def ready.
setInsertPt(*CurMBB, std::next(MII));
} else if (!dominates(MI, CurrPos)) {
// Update the spliced machineinstr's debug location by merging it with the
// debug location of the instruction at the insertion point.
auto *Loc = DILocation::getMergedLocation(getDebugLoc().get(),
MI->getDebugLoc().get());
MI->setDebugLoc(Loc);
CurMBB->splice(CurrPos, CurMBB, MI);
}
return MachineInstrBuilder(getMF(), MI);
Expand Down
30 changes: 30 additions & 0 deletions llvm/test/DebugInfo/AArch64/merge-locations-legalizer.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# This test checks to make sure that when an instruction (%3 in the test) is
# moved due to matching a result of a fold of two other instructions
# (%1, and %2 in the test) in the legalizer, the DILocation of the
# instruction that is moved (%3) is updated appropriately.

# RUN: llc %s -run-pass=legalizer -mtriple=aarch64 -o - | FileCheck %s
# CHECK-NOT: %2:_(s32) = G_CONSTANT i32 0, debug-location !DILocation(line: 13
# CHECK: %2:_(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: "main.swift", directory: "/Volumes/Data/swift")
!23 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !3, sdk: "blah.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
body: |
bb.0:
%1:_(s8) = G_CONSTANT i8 0, debug-location !68
%2:_(s32) = G_ANYEXT %1(s8), debug-location !68
$w2 = COPY %2(s32), debug-location !68
%3:_(s32) = G_CONSTANT i32 0, debug-location !71
$w0 = COPY %3(s32), debug-location !71

0 comments on commit a9763de

Please sign in to comment.