Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CodeGen][DebugInfo] Add missing DebugLoc for SplitCriticalEdge #72192

Merged
merged 2 commits into from
Apr 8, 2024

Conversation

HaohaiWen
Copy link
Contributor

In SplitCriticalEdge, DebugLoc of the branch instruction in new created
MBB was set to empty. It should be set and we can find proper DebugLoc
for it in most cases. This patch set it to non empty merged DebugLoc of
current MBB branches.

In SplitCriticalEdge, DebugLoc of the branch instruction in new created
MBB was set to empty. It should be set and we can find proper DebugLoc
for it in most cases. This patch set it to non empty merged DebugLoc of
current MBB branches.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 14, 2023

@llvm/pr-subscribers-backend-x86

Author: None (HaohaiWen)

Changes

In SplitCriticalEdge, DebugLoc of the branch instruction in new created
MBB was set to empty. It should be set and we can find proper DebugLoc
for it in most cases. This patch set it to non empty merged DebugLoc of
current MBB branches.


Full diff: https://github.com/llvm/llvm-project/pull/72192.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+9-1)
  • (modified) llvm/test/CodeGen/X86/fsafdo_test1.ll (+1-1)
diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index d9e22685faf5f5e..9c6a77f34314f42 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -1137,7 +1137,6 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
 
   MachineFunction *MF = getParent();
   MachineBasicBlock *PrevFallthrough = getNextNode();
-  DebugLoc DL;  // FIXME: this is nowhere
 
   MachineBasicBlock *NMBB = MF->CreateMachineBasicBlock();
   NMBB->setCallFrameSize(Succ->getCallFrameSize());
@@ -1218,6 +1217,15 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
     SlotIndexUpdateDelegate SlotUpdater(*MF, Indexes);
     SmallVector<MachineOperand, 4> Cond;
     const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();
+
+    // In original 'this' BB, there must be a branch instruction targeting at
+    // Succ. We can not find it out since currently getBranchDestBlock was not
+    // implemented for all targets. However, if the merged DL has column or line
+    // number, the scope and non-zero column and line number is same with that
+    // branch instruction so we can safely use it.
+    DebugLoc DL, MergedDL = findBranchDebugLoc();
+    if (MergedDL && (MergedDL.getLine() || MergedDL.getCol()))
+      DL = MergedDL;
     TII->insertBranch(*NMBB, Succ, nullptr, Cond, DL);
   }
 
diff --git a/llvm/test/CodeGen/X86/fsafdo_test1.ll b/llvm/test/CodeGen/X86/fsafdo_test1.ll
index b5ae3915294cd7a..61c0f59aba6f818 100644
--- a/llvm/test/CodeGen/X86/fsafdo_test1.ll
+++ b/llvm/test/CodeGen/X86/fsafdo_test1.ll
@@ -6,7 +6,7 @@
 ; V01: .loc    1 9 5 is_stmt 1 discriminator 2 # foo.c:9:5
 ; V0: .loc    1 9 5 is_stmt 0 discriminator 11266 # foo.c:9:5
 ; V0: .loc    1 7 3 is_stmt 1 discriminator 11266 # foo.c:7:3
-; V1: .loc    1 9 5 is_stmt 0 discriminator 258 # foo.c:9:5
+; V1: .loc    1 9 5 is_stmt 0 discriminator 514 # foo.c:9:5
 ; V1: .loc    1 7 3 is_stmt 1 discriminator 258 # foo.c:7:3
 ; Check that variable __llvm_fs_discriminator__ is generated.
 ; V01: .type   __llvm_fs_discriminator__,@object # @__llvm_fs_discriminator__

@@ -6,7 +6,7 @@
; V01: .loc 1 9 5 is_stmt 1 discriminator 2 # foo.c:9:5
; V0: .loc 1 9 5 is_stmt 0 discriminator 11266 # foo.c:9:5
; V0: .loc 1 7 3 is_stmt 1 discriminator 11266 # foo.c:7:3
; V1: .loc 1 9 5 is_stmt 0 discriminator 258 # foo.c:9:5
; V1: .loc 1 9 5 is_stmt 0 discriminator 514 # foo.c:9:5
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before SplitCriticalEdge:

bb.1.while.cond1.preheader.lr.ph:
; predecessors: %bb.0
  successors: %bb.3(0x30000000), %bb.5(0x50000000); %bb.3(37.50%), %bb.5(62.50%)

  TEST64rr %0:gr64, %0:gr64, implicit-def $eflags
  JCC_1 %bb.3, 4, implicit $eflags, debug-location !10; foo.c:9:5
  JMP_1 %bb.5, debug-location !10; foo.c:9:5
....
bb.5.while.body2:
; predecessors: %bb.5, %bb.1
  successors: %bb.5(0x80000000); %bb.5(100.00%)

  JMP_1 %bb.5, debug-location !10; foo.c:9:5

After SplitCriticalEdge without this PR:

bb.1.while.cond1.preheader.lr.ph:
; predecessors: %bb.0
  successors: %bb.3(0x30000000), %bb.7(0x50000000); %bb.3(37.50%), %bb.7(62.50%)

  TEST64rr %0:gr64, %0:gr64, implicit-def $eflags
  JCC_1 %bb.3, 4, implicit $eflags, debug-location !10; foo.c:9:5

bb.7:
; predecessors: %bb.1
  successors: %bb.5(0x80000000); %bb.5(100.00%)

  JMP_1 %bb.5
...
bb.5.while.body2:
; predecessors: %bb.5, %bb.7
  successors: %bb.5(0x80000000); %bb.5(100.00%)

  JMP_1 %bb.5, debug-location !10; foo.c:9:5

After SplitCriticalEdge with this PR:

; predecessors: %bb.0
  successors: %bb.3(0x30000000), %bb.7(0x50000000); %bb.3(37.50%), %bb.7(62.50%)

  TEST64rr %0:gr64, %0:gr64, implicit-def $eflags
  JCC_1 %bb.3, 4, implicit $eflags, debug-location !10; foo.c:9:5

bb.7:
; predecessors: %bb.1
  successors: %bb.5(0x80000000); %bb.5(100.00%)

  JMP_1 %bb.5, debug-location !10; foo.c:9:5
...
bb.5.while.body2:
; predecessors: %bb.5, %bb.7
  successors: %bb.5(0x80000000); %bb.5(100.00%)

  JMP_1 %bb.5, debug-location !10; foo.c:9:5

Copy link
Contributor

@williamweixiao williamweixiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@HaohaiWen HaohaiWen merged commit cebf77f into llvm:main Apr 8, 2024
4 checks passed
@HaohaiWen HaohaiWen deleted the debuginfo branch April 8, 2024 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants