[llvm] ec9da51 - [DebugInfo][InstrRef] Correctly update DBG_PHIs during instr scheduling
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 27 07:13:19 PDT 2021
Author: Jeremy Morse
Date: 2021-07-27T15:12:46+01:00
New Revision: ec9da5172491413f098af7cd5b5bc5d1c8b9f07d
URL: https://github.com/llvm/llvm-project/commit/ec9da5172491413f098af7cd5b5bc5d1c8b9f07d
DIFF: https://github.com/llvm/llvm-project/commit/ec9da5172491413f098af7cd5b5bc5d1c8b9f07d.diff
LOG: [DebugInfo][InstrRef] Correctly update DBG_PHIs during instr scheduling
Avoid several crashes when DBG_INSTR_REF and DBG_PHI instructions are fed
to the instruction scheduler. DBG_INSTR_REFs should be treated like
DBG_LABELs, and just ignored for the purpose of scheduling [0].
DBG_PHIs however behave much more like DBG_VALUEs: they refer to register
operands, and if some register defs get shuffled around during instruction
scheduling, there's a risk that the debug instr will refer to the wrong
value. There's already a facility for updating DBG_VALUEs to reflect this;
add DBG_PHI to the list of instructions that it will update.
[0] Suboptimal, but it's what instr scheduling does right now.
Differential Revision: https://reviews.llvm.org/D106663
Added:
Modified:
llvm/include/llvm/CodeGen/AntiDepBreaker.h
llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/AntiDepBreaker.h b/llvm/include/llvm/CodeGen/AntiDepBreaker.h
index 4b8fb4c5fbde..c5c2b5748613 100644
--- a/llvm/include/llvm/CodeGen/AntiDepBreaker.h
+++ b/llvm/include/llvm/CodeGen/AntiDepBreaker.h
@@ -55,13 +55,20 @@ class AntiDepBreaker {
/// 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
diff --git a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
index 2a4b0476b8b7..daff3af3bc3c 100644
--- a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
+++ b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
@@ -807,14 +807,12 @@ void ScheduleDAGInstrs::buildSchedGraph(AAResults *AA,
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];
diff --git a/llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir b/llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir
index 98d25a5c41b8..261d6b9ecb74 100644
--- a/llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir
+++ b/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 @@ body: |
; 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 @@ body: |
$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
More information about the llvm-commits
mailing list