[llvm] r352350 - [DebugInfo][DAG] Avoid re-ordering of DBG_VALUEs
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 28 04:08:31 PST 2019
Author: jmorse
Date: Mon Jan 28 04:08:31 2019
New Revision: 352350
URL: http://llvm.org/viewvc/llvm-project?rev=352350&view=rev
Log:
[DebugInfo][DAG] Avoid re-ordering of DBG_VALUEs
This patch improves the placement of DBG_VALUEs when by SelectionDAG, which
as documented in PR40427 can go very wrong. At the core of this is
ProcessSourceNode, which assumes the last instruction in a BB is the start
of the last processed IR instruction, which isn't always true.
Instead, use a helper function to call InstrEmitter::EmitNode, that records
before-and-after iterators and determines the first of any new instruction
created during emission. This is passed to ProcessSourceNode, which can
then make more elightened decisions about ordering for DBG_VALUE placement.
Differential revision: https://reviews.llvm.org/D57163
Added:
llvm/trunk/test/DebugInfo/X86/pr40427.ll
Modified:
llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll
llvm/trunk/test/DebugInfo/X86/sdag-dbgvalue-phi-use-2.ll
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp?rev=352350&r1=352349&r2=352350&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp Mon Jan 28 04:08:31 2019
@@ -740,28 +740,27 @@ ProcessSDDbgValues(SDNode *N, SelectionD
static void
ProcessSourceNode(SDNode *N, SelectionDAG *DAG, InstrEmitter &Emitter,
DenseMap<SDValue, unsigned> &VRBaseMap,
- SmallVectorImpl<std::pair<unsigned, MachineInstr*> > &Orders,
- SmallSet<unsigned, 8> &Seen) {
+ SmallVectorImpl<std::pair<unsigned, MachineInstr *>> &Orders,
+ SmallSet<unsigned, 8> &Seen, MachineInstr *NewInsn) {
unsigned Order = N->getIROrder();
- if (!Order || !Seen.insert(Order).second) {
+ if (!Order || Seen.count(Order)) {
// Process any valid SDDbgValues even if node does not have any order
// assigned.
ProcessSDDbgValues(N, DAG, Emitter, Orders, VRBaseMap, 0);
return;
}
- MachineBasicBlock *BB = Emitter.getBlock();
- auto IP = Emitter.getInsertPos();
- if (IP == BB->begin() || BB->back().isPHI() ||
- // Fast-isel may have inserted some instructions, in which case the
- // BB->back().isPHI() test will not fire when we want it to.
- std::prev(IP)->isPHI()) {
- // Did not insert any instruction.
- Orders.push_back({Order, (MachineInstr *)nullptr});
- return;
+ // If a new instruction was generated for this Order number, record it.
+ // Otherwise, leave this order number unseen: we will either find later
+ // instructions for it, or leave it unseen if there were no instructions at
+ // all.
+ if (NewInsn) {
+ Seen.insert(Order);
+ Orders.push_back({Order, NewInsn});
}
- Orders.push_back({Order, &*std::prev(IP)});
+ // Even if no instruction was generated, a Value may have become defined via
+ // earlier nodes. Try to process them now.
ProcessSDDbgValues(N, DAG, Emitter, Orders, VRBaseMap, Order);
}
@@ -814,6 +813,37 @@ EmitSchedule(MachineBasicBlock::iterator
SmallSet<unsigned, 8> Seen;
bool HasDbg = DAG->hasDebugValues();
+ // Emit a node, and determine where its first instruction is for debuginfo.
+ // Zero, one, or multiple instructions can be created when emitting a node.
+ auto EmitNode =
+ [&](SDNode *Node, bool IsClone, bool IsCloned,
+ DenseMap<SDValue, unsigned> &VRBaseMap) -> MachineInstr * {
+ // Fetch instruction prior to this, or end() if nonexistant.
+ auto GetPrevInsn = [&](MachineBasicBlock::iterator I) {
+ if (I == BB->begin())
+ return BB->end();
+ else
+ return std::prev(Emitter.getInsertPos());
+ };
+
+ MachineBasicBlock::iterator Before = GetPrevInsn(Emitter.getInsertPos());
+ Emitter.EmitNode(Node, IsClone, IsCloned, VRBaseMap);
+ MachineBasicBlock::iterator After = GetPrevInsn(Emitter.getInsertPos());
+
+ // If the iterator did not change, no instructions were inserted.
+ if (Before == After)
+ return nullptr;
+
+ if (Before == BB->end()) {
+ // There were no prior instructions; the new ones must start at the
+ // beginning of the block.
+ return &Emitter.getBlock()->instr_front();
+ } else {
+ // Return first instruction after the pre-existing instructions.
+ return &*std::next(Before);
+ }
+ };
+
// If this is the first BB, emit byval parameter dbg_value's.
if (HasDbg && BB->getParent()->begin() == MachineFunction::iterator(BB)) {
SDDbgInfo::DbgIterator PDI = DAG->ByvalParmDbgBegin();
@@ -850,18 +880,18 @@ EmitSchedule(MachineBasicBlock::iterator
GluedNodes.push_back(N);
while (!GluedNodes.empty()) {
SDNode *N = GluedNodes.back();
- Emitter.EmitNode(N, SU->OrigNode != SU, SU->isCloned, VRBaseMap);
+ auto NewInsn = EmitNode(N, SU->OrigNode != SU, SU->isCloned, VRBaseMap);
// Remember the source order of the inserted instruction.
if (HasDbg)
- ProcessSourceNode(N, DAG, Emitter, VRBaseMap, Orders, Seen);
+ ProcessSourceNode(N, DAG, Emitter, VRBaseMap, Orders, Seen, NewInsn);
GluedNodes.pop_back();
}
- Emitter.EmitNode(SU->getNode(), SU->OrigNode != SU, SU->isCloned,
- VRBaseMap);
+ auto NewInsn =
+ EmitNode(SU->getNode(), SU->OrigNode != SU, SU->isCloned, VRBaseMap);
// Remember the source order of the inserted instruction.
if (HasDbg)
- ProcessSourceNode(SU->getNode(), DAG, Emitter, VRBaseMap, Orders,
- Seen);
+ ProcessSourceNode(SU->getNode(), DAG, Emitter, VRBaseMap, Orders, Seen,
+ NewInsn);
}
// Insert all the dbg_values which have not already been inserted in source
@@ -886,8 +916,7 @@ EmitSchedule(MachineBasicBlock::iterator
unsigned Order = Orders[i].first;
MachineInstr *MI = Orders[i].second;
// Insert all SDDbgValue's whose order(s) are before "Order".
- if (!MI)
- continue;
+ assert(MI);
for (; DI != DE; ++DI) {
if ((*DI)->getOrder() < LastOrder || (*DI)->getOrder() >= Order)
break;
Modified: llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll?rev=352350&r1=352349&r2=352350&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll (original)
+++ llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll Mon Jan 28 04:08:31 2019
@@ -4781,8 +4781,8 @@ if.end:
; CHECK-NEXT: .b8 6 // DW_AT_call_line
; CHECK-NEXT: .b8 43 // Abbrev [43] 0x270e:0x22 DW_TAG_inlined_subroutine
; CHECK-NEXT: .b32 9791 // DW_AT_abstract_origin
-; CHECK-NEXT: .b64 Ltmp8 // DW_AT_low_pc
-; CHECK-NEXT: .b64 Ltmp9 // DW_AT_high_pc
+; CHECK-NEXT: .b64 Ltmp9 // DW_AT_low_pc
+; CHECK-NEXT: .b64 Ltmp10 // DW_AT_high_pc
; CHECK-NEXT: .b8 12 // DW_AT_call_file
; CHECK-NEXT: .b8 8 // DW_AT_call_line
; CHECK-NEXT: .b8 44 // Abbrev [44] 0x2725:0x5 DW_TAG_formal_parameter
Added: llvm/trunk/test/DebugInfo/X86/pr40427.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/pr40427.ll?rev=352350&view=auto
==============================================================================
--- llvm/trunk/test/DebugInfo/X86/pr40427.ll (added)
+++ llvm/trunk/test/DebugInfo/X86/pr40427.ll Mon Jan 28 04:08:31 2019
@@ -0,0 +1,46 @@
+; RUN: llc -start-after=codegenprepare -stop-before=expand-isel-pseudos -o - < %s | FileCheck %s
+; Test for correct placement of DBG_VALUE, which in PR40427 is placed before
+; the load instruction it refers to. The circumstance replicated here is where
+; two instructions in a row, trunc and add, begin with no-op Copy{To,From}Reg
+; SDNodes that produce no instructions.
+; The DBG_VALUE instruction should come immediately after the load instruction
+; because the truncate is optimised out, and the DBG_VALUE should be placed
+; in front of the first instruction that occurs after the dbg.value.
+
+; CHECK: ![[DBGVAR:[0-9]+]] = !DILocalVariable(name: "bees",
+
+define i16 @lolwat(i1 %spoons, i64 *%bees, i16 %yellow, i64 *%more) {
+entry:
+ br i1 %spoons, label %trueb, label %falseb
+trueb:
+ br label %block
+falseb:
+ br label %block
+block:
+; CHECK: [[PHIREG:%[0-9]+]]:gr64 = PHI %6, %bb.2, %4, %bb.1
+; CHECK-NEXT: [[LOADR:%[0-9]+]]:gr16 = MOV16rm %0
+; CHECK-NEXT: DBG_VALUE [[LOADR]], $noreg, ![[DBGVAR]]
+; CHECK-NEXT: %{{[0-9]+}}:gr32 = IMPLICIT_DEF
+ %foo = phi i64 *[%bees, %trueb], [%more, %falseb]
+ %forks = bitcast i64 *%foo to i32 *
+ %ret = load i32, i32 *%forks, !dbg !6
+ %cast = trunc i32 %ret to i16, !dbg !6
+ call void @llvm.dbg.value(metadata i16 %cast, metadata !1, metadata !DIExpression()), !dbg !6
+ %orly2 = add i16 %yellow, 1
+ br label %bb1
+bb1:
+ %cheese = add i16 %orly2, %cast
+ ret i16 %cheese, !dbg !6
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.module.flags = !{!4}
+!llvm.dbg.cu = !{!2}
+!1 = !DILocalVariable(name: "bees", scope: !5, type: null)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "beards", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug)
+!3 = !DIFile(filename: "bees.cpp", directory: "")
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "nope", scope: !2, file: !3, line: 1, unit: !2)
+!6 = !DILocation(line: 0, scope: !5)
+!7 = !DILocalVariable(name: "flannel", scope: !5, type: null)
Modified: llvm/trunk/test/DebugInfo/X86/sdag-dbgvalue-phi-use-2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/sdag-dbgvalue-phi-use-2.ll?rev=352350&r1=352349&r2=352350&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/X86/sdag-dbgvalue-phi-use-2.ll (original)
+++ llvm/trunk/test/DebugInfo/X86/sdag-dbgvalue-phi-use-2.ll Mon Jan 28 04:08:31 2019
@@ -44,15 +44,17 @@ for.body:
; CHECK-NEXT: [[REG4:%[0-9]+]]:gr32 = PHI
; CHECK-NEXT: DBG_VALUE [[REG3]], $noreg, !16
; CHECK-NEXT: DBG_VALUE 555, $noreg, !17
-; XXX: Shouldn't the following DBG_VALUE be placed after the add (ADD32rr).
+; CHECK-NEXT: [[ADDREG:%[0-9]+]]:gr32 = nuw nsw ADD32rr
; CHECK-NEXT: DBG_VALUE [[REG2]], $noreg, !17
-; CHECK-NEXT: ADD32rr
-; XXX: Shouldn't the following DBG_VALUE be placed after the mul (LEA etc).
+; CHECK: [[MULREG:%[0-9]+]]:gr32 = LEA64_32r
; CHECK-NEXT: DBG_VALUE 777, $noreg, !17
-; CHECK: INC32r
-; XXX: Shouldn't the following DBG_VALUE be placed after the icmp (the non-dead implicit def of $eflags)
-; CHECK: DBG_VALUE [[REG4]]
-; CHECK-NEXT: implicit-def $eflags
+; XXX: The following DBG_VALUE should have stayed below the INC32r
+; CHECK-NEXT: DBG_VALUE [[MULREG]], $noreg, !16
+; CHECK-NEXT: [[INCREG:%[0-9]+]]:gr32 = nuw nsw INC32r
+; CHECK-NEXT: DBG_VALUE [[INCREG]], $noreg, !17
+; CHECK-NEXT: DBG_VALUE [[ADDREG]], $noreg, !15
+; CHECK-NEXT: implicit-def $eflags,
+; CHECK-NEXT: DBG_VALUE [[REG4]]
%u.023 = phi i32 [ 0, %for.body.lr.ph ], [ %inc, %for.body ]
%y.022 = phi i32 [ 13, %for.body.lr.ph ], [ %mul, %for.body ]
%x.021 = phi i32 [ 9, %for.body.lr.ph ], [ %add, %for.body ]
More information about the llvm-commits
mailing list