Skip to content

Commit

Permalink
[ScheduleDAG] Move DBG_VALUEs after first term forward.
Browse files Browse the repository at this point in the history
MBBs are not allowed to have non-terminator instructions after the first
terminator. Currently in some cases (see the modified test),
EmitSchedule can add DBG_VALUEs after the last terminator, for example
when referring a debug value that gets folded into a TCRETURN
instruction on ARM.

This patch updates EmitSchedule to move inserted DBG_VALUEs just before
the first terminator. I am not sure if there are terminators produce
values that can in turn be used by a DBG_VALUE. In that case, moving the
DBG_VALUE might result in referencing an undefined register. But in any
case, it seems like currently there is no way to insert a proper DBG_VALUEs
for such registers anyways.

Alternatively it might make sense to just remove those extra DBG_VALUES.

I am not too familiar with the details of debug info in the backend and
would appreciate any suggestions on how to address the issue in the best
possible way.

Reviewers: vsk, aprantl, jpaquette, efriedma, paquette

Reviewed By: aprantl

Differential Revision: https://reviews.llvm.org/D83561
  • Loading branch information
fhahn committed Jul 17, 2020
1 parent 650baf2 commit e297006
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 4 deletions.
3 changes: 3 additions & 0 deletions llvm/include/llvm/CodeGen/MachineInstr.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ class MachineInstr
const MachineBasicBlock* getParent() const { return Parent; }
MachineBasicBlock* getParent() { return Parent; }

/// Move the instruction before \p MovePos.
void moveBefore(MachineInstr *MovePos);

/// Return the function that contains the basic block that this instruction
/// belongs to.
///
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/CodeGen/MachineInstr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ MachineInstr::MachineInstr(MachineFunction &MF, const MachineInstr &MI)
setFlags(MI.Flags);
}

void MachineInstr::moveBefore(MachineInstr *MovePos) {
MovePos->getParent()->splice(MovePos, getParent(), getIterator());
}

/// getRegInfo - If this instruction is embedded into a MachineFunction,
/// return the MachineRegisterInfo object for the current function, otherwise
/// return null.
Expand Down
24 changes: 23 additions & 1 deletion llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,29 @@ EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
}

InsertPos = Emitter.getInsertPos();
return Emitter.getBlock();
// In some cases, DBG_VALUEs might be inserted after the first terminator,
// which results in an invalid MBB. If that happens, move the DBG_VALUEs
// before the first terminator.
MachineBasicBlock *InsertBB = Emitter.getBlock();
auto FirstTerm = InsertBB->getFirstTerminator();
if (FirstTerm != InsertBB->end()) {
assert(!FirstTerm->isDebugValue() &&
"first terminator cannot be a debug value");
for (MachineInstr &MI : make_early_inc_range(
make_range(std::next(FirstTerm), InsertBB->end()))) {
if (!MI.isDebugValue())
continue;

if (&MI == InsertPos)
InsertPos = std::prev(InsertPos->getIterator());

// The DBG_VALUE was referencing a value produced by a terminator. By
// moving the DBG_VALUE, the referenced value also needs invalidating.
MI.getOperand(0).ChangeToRegister(0, false);
MI.moveBefore(&*FirstTerm);
}
}
return InsertBB;
}

/// Return the basic block label.
Expand Down
5 changes: 2 additions & 3 deletions llvm/test/CodeGen/ARM/dbg-tcreturn.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
; XFAIL: *
; RUN: llc %s -o - -stop-after=finalize-isel -verify-machineinstr | FileCheck %s
; RUN: llc %s -o - -stop-after=finalize-isel -verify-machineinstrs | FileCheck %s

target datalayout = "e-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
target triple = "thumbv7-apple-ios7.0.0"
Expand All @@ -12,8 +11,8 @@ target triple = "thumbv7-apple-ios7.0.0"
; CHECK-NEXT: %0:gpr = COPY $r0
; CHECK-NEXT: $r0 = COPY %0
; CHECK-NEXT: $r1 = COPY %1
; CHECK-NEXT: TCRETURNdi &__divsi3, implicit $sp, implicit $r0, implicit $r1
; CHECK-NEXT: DBG_VALUE $noreg, $noreg, !13, !DIExpression(), debug-location !16
; CHECK-NEXT: TCRETURNdi &__divsi3, implicit $sp, implicit $r0, implicit $r1

define i32 @test(i32 %a1, i32 %a2) !dbg !5 {
entry:
Expand Down

0 comments on commit e297006

Please sign in to comment.