[PATCH] D83561: [ScheduleDAG] Move DBG_VALUEs after first term forward.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: vsk, aprantl, jpaquette, efriedma.
Herald added subscribers: hiraditya, kristof.beyls, MatzeB.
Herald added a reviewer: paquette.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83561

Files:
  llvm/include/llvm/CodeGen/MachineInstr.h
  llvm/lib/CodeGen/MachineInstr.cpp
  llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
  llvm/test/CodeGen/ARM/dbg-tcreturn.ll


Index: llvm/test/CodeGen/ARM/dbg-tcreturn.ll
===================================================================
--- llvm/test/CodeGen/ARM/dbg-tcreturn.ll
+++ llvm/test/CodeGen/ARM/dbg-tcreturn.ll
@@ -1,4 +1,4 @@
-; RUN: llc %s -o - -stop-after=finalize-isel | 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"
@@ -11,8 +11,8 @@
 ; 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:
Index: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
+++ llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
@@ -1034,7 +1034,25 @@
   }
 
   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());
+      MI.moveBefore(&*FirstTerm);
+    }
+  }
+  return InsertBB;
 }
 
 /// Return the basic block label.
Index: llvm/lib/CodeGen/MachineInstr.cpp
===================================================================
--- llvm/lib/CodeGen/MachineInstr.cpp
+++ llvm/lib/CodeGen/MachineInstr.cpp
@@ -147,6 +147,10 @@
   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.
Index: llvm/include/llvm/CodeGen/MachineInstr.h
===================================================================
--- llvm/include/llvm/CodeGen/MachineInstr.h
+++ llvm/include/llvm/CodeGen/MachineInstr.h
@@ -280,6 +280,9 @@
   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.
   ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83561.277058.patch
Type: text/x-patch
Size: 3132 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200710/97ba82d8/attachment.bin>