[PATCH] D106663: [DebugInfo][InstrRef] Correctly update DBG_PHIs during instruction scheduling
Jeremy Morse via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 23 07:13:14 PDT 2021
jmorse created this revision.
jmorse added reviewers: Orlando, TWeaver, StephenTozer.
Herald added subscribers: pengfei, hiraditya, MatzeB.
jmorse requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
When the instruction scheduler runs, it saves debug-instructions, and when writing the instruction referencing stuff I figured that the new debug instructions should be treated the same as DBG_VALUEs. However, it turns out there's a nuance:
- DBG_VALUEs can have their operands updated by the instruction scheduler, if it determines that the movement of an instruction changes what the DBG_VALUE refers to,
- DBG_LABELs and other unimportant meta instructions are just ignored.
This is a feature I was unaware of, and it's great; unfortunately it causes a crash with instruction referencing, because:
- DBG_PHIs should be treated like DBG_VALUEs, and have their operands updated,
- DBG_INSTR_REFs should be treated like DBG_LABELs, just ignored.
This patch rectifies that: DBG_PHIs are placed in the collection of instructions that need to have their operands updated, and `UpdateDbgValue` now deals with DBG_PHIs too. In `ScheduleDAGInstrs::buildSchedGraph`, DBG_INSTR_REFs are now treated like DBG_LABELs and are ignored for instruction scheduling purposes. The modified test checks the same behaviour for DBG_VALUEs and DBG_PHIs, that when a register is changed in scheduling (rcx -> r8), both DBG_VALUEs and DBG_PHIs are updated.
(Someday I think we'll need to have the instruction scheduler become more debug-info aware, to avoid re-ordering assignments, but it is not that day).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D106663
Files:
llvm/include/llvm/CodeGen/AntiDepBreaker.h
llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir
Index: llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir
===================================================================
--- llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir
+++ llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir
@@ -1,10 +1,12 @@
# RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=btver2 -run-pass=post-RA-sched -o - %s | FileCheck %s
+# RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=btver2 -run-pass=post-RA-sched -o - %s -experimental-debug-variable-locations| FileCheck %s
-# Test that multiple DBG_VALUE's following an instruction whose register needs
-# to be changed during the post-RA scheduler pass are updated correctly.
+# Test that multiple DBG_VALUE's and DBG_PHIs following an instruction whose
+# register needs # to be changed during the post-RA scheduler pass are updated
+# correctly.
# Test case was derived from the output from the following command and
-# the source code below:
+# the source code below. DBG_PHIs added manually later:
#
# clang -S -emit-llvm -target x86_64 -march=btver2 -O2 -g -o - <srcfile> |
# llc -stop-before=post-RA-sched -o -
@@ -253,6 +255,8 @@
; CHECK: [[REGISTER:\$r[a-z0-9]+]] = LEA64r {{\$r[a-z0-9]+}}, 1, $noreg, -20, $noreg
; CHECK-NEXT: DBG_VALUE [[REGISTER]], $noreg, ![[J_VAR]], !DIExpression(), debug-location ![[J_LOC]]
; CHECK-NEXT: DBG_VALUE [[REGISTER]], $noreg, ![[I_VAR]], !DIExpression(), debug-location ![[I_LOC]]
+ ; CHECK-NEXT: DBG_PHI [[REGISTER]], 0
+ ; CHECK-NEXT: DBG_PHI [[REGISTER]], 1
frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp
CFI_INSTRUCTION def_cfa_offset 16
@@ -288,10 +292,14 @@
$rcx = LEA64r $rbp, 1, $noreg, -20, $noreg
DBG_VALUE $rcx, $noreg, !46, !17, debug-location !48
DBG_VALUE $rcx, $noreg, !39, !17, debug-location !44
+ DBG_PHI $rcx, 0
+ DBG_PHI $rcx, 1
DBG_VALUE $rbp, -20, !29, !17, debug-location !36
$rcx = CMOV64rr killed $rcx, killed $rdx, 5, implicit killed $eflags
$rcx = OR64rr killed $rcx, killed $rsi, implicit-def dead $eflags
$rdx = MOVSX64rm32 $rbx, 1, $noreg, 0, $noreg :: (load (s32), align 8)
+ DBG_INSTR_REF 1, 0, !46, !17, debug-location !48
+ DBG_INSTR_REF 2, 0, !39, !17, debug-location !44
TEST32mr killed $rcx, 4, killed $rdx, 0, $noreg, killed $eax, implicit-def $eflags :: (load (s32))
JCC_1 %bb.2, 5, implicit $eflags
JMP_1 %bb.3
Index: llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
===================================================================
--- llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
+++ llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
@@ -807,14 +807,12 @@
DbgMI = nullptr;
}
- if (MI.isDebugValue() || MI.isDebugRef() || MI.isDebugPHI()) {
+ if (MI.isDebugValue() || MI.isDebugPHI()) {
DbgMI = &MI;
continue;
}
- if (MI.isDebugLabel())
- continue;
- if (MI.isPseudoProbe())
+ if (MI.isDebugLabel() || MI.isDebugRef() || MI.isPseudoProbe())
continue;
SUnit *SU = MISUnitMap[&MI];
Index: llvm/include/llvm/CodeGen/AntiDepBreaker.h
===================================================================
--- llvm/include/llvm/CodeGen/AntiDepBreaker.h
+++ llvm/include/llvm/CodeGen/AntiDepBreaker.h
@@ -55,13 +55,20 @@
/// Finish anti-dep breaking for a basic block.
virtual void FinishBlock() = 0;
- /// Update DBG_VALUE if dependency breaker is updating
+ /// Update DBG_VALUE or DBG_PHI if dependency breaker is updating
/// other machine instruction to use NewReg.
void UpdateDbgValue(MachineInstr &MI, unsigned OldReg, unsigned NewReg) {
- assert(MI.isDebugValue() && "MI is not DBG_VALUE!");
- if (MI.getDebugOperand(0).isReg() &&
- MI.getDebugOperand(0).getReg() == OldReg)
- MI.getDebugOperand(0).setReg(NewReg);
+ if (MI.isDebugValue()) {
+ if (MI.getDebugOperand(0).isReg() &&
+ MI.getDebugOperand(0).getReg() == OldReg)
+ MI.getDebugOperand(0).setReg(NewReg);
+ } else if (MI.isDebugPHI()) {
+ if (MI.getOperand(0).isReg() &&
+ MI.getOperand(0).getReg() == OldReg)
+ MI.getOperand(0).setReg(NewReg);
+ } else {
+ llvm_unreachable("MI is not DBG_VALUE / DBG_PHI!");
+ }
}
/// Update all DBG_VALUE instructions that may be affected by the dependency
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D106663.361184.patch
Type: text/x-patch
Size: 4290 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210723/8eedbee0/attachment.bin>
More information about the llvm-commits
mailing list