[PHIEliminate] Move dbg values after phi and label

If there are DBG_VALUEs between phi and label (after phi and before label),
DBG_VALUE will block PHI lowering after the LABEL. Moving all DBG_VALUEs
after Labels in the function ScheduleDAGSDNodes::EmitSchedule to avoid
impacting PHI lowering.

  before:
     PHI
     DBG_VALUE
     LABEL
  after: (move DBG_VALUE after label)
     PHI
     LABEL
     DBG_VALUE
  then: (phi lowering after label)
     LABEL
     COPY
     DBG_VALUE

Fixes the issue: https://bugs.llvm.org/show_bug.cgi?id=43859

Differential Revision: https://reviews.llvm.org/D70597

diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 6c0402df84892066412679911c2bf3afe02f5f93..24fb330bafdddb42b906fd13279a487e126f842b 100644 (file)
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -273,6 +273,7 @@ namespace {
     void markReachable(const MachineBasicBlock *MBB);
     void calcRegsPassed();
     void checkPHIOps(const MachineBasicBlock &MBB);
+    void checkPHIDbg(const MachineBasicBlock &MBB);
 
     void calcRegsRequired();
     void verifyLiveVariables();
@@ -2262,11 +2263,31 @@ void MachineVerifier::checkPHIOps(const MachineBasicBlock &MBB) {
   }
 }
 
+// Check there must be no DBG_VALUEs between PHIs and LABELs, so that
+// the PHIs could lower after LABELs successfully without dbg barriers.
+void MachineVerifier::checkPHIDbg(const MachineBasicBlock &MBB) {
+
+  // Quick exit for basic blocks without PHIs.
+  if (MBB.empty() || !MBB.front().isPHI())
+    return;
+
+  MachineBasicBlock &MBB2 = const_cast<MachineBasicBlock &>(MBB);
+  MachineBasicBlock::iterator LastPHIIt =
+    std::prev(MBB2.SkipPHIsAndLabels(MBB2.begin()));
+  // Backward to skip debug if the lastPHIIt is DBG_VALUE, the LastPHIIt
+  // should be the same if no DBG_VALUEs between PHIs and LABELs.
+  if (LastPHIIt != skipDebugInstructionsBackward(
+          std::prev(MBB2.SkipPHIsLabelsAndDebug(MBB2.begin())), MBB2.begin()))
+    report("Unexpected DBG_VALUEs between PHI and LABEL", &MBB2);
+}
+
 void MachineVerifier::visitMachineFunctionAfter() {
   calcRegsPassed();
 
-  for (const MachineBasicBlock &MBB : *MF)
+  for (const MachineBasicBlock &MBB : *MF) {
     checkPHIOps(MBB);
+    checkPHIDbg(MBB);
+  }
 
   // Now check liveness info if available
   calcRegsRequired();

diff --git a/llvm/lib/CodeGen/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp
index 4dd4c4b1084e59db1fe6f354db6b561622dce5fd..1ed479db54d3bb22ce14b28b2998fc8dabde90d2 100644 (file)
--- a/llvm/lib/CodeGen/PHIElimination.cpp
+++ b/llvm/lib/CodeGen/PHIElimination.cpp
@@ -210,6 +210,11 @@ bool PHIElimination::EliminatePHINodes(MachineFunction &MF,
   MachineBasicBlock::iterator LastPHIIt =
     std::prev(MBB.SkipPHIsAndLabels(MBB.begin()));
 
+  assert((LastPHIIt == skipDebugInstructionsBackward(
+            std::prev(MBB.SkipPHIsLabelsAndDebug(MBB.begin())),
+            MBB.begin())) &&
+         "There must be no DBG_VALUEs between PHI and LABEL");
+
   while (MBB.front().isPHI())
     LowerPHINode(MBB, LastPHIIt);
 

diff --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
index 0e4d783e3505d61e1985c71591800598caa967d0..a911aecb352e7b2dd610c398301c8c95fb41c10e 100644 (file)
--- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
@@ -929,10 +929,12 @@ EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
     }
   }
 
-  // Insert all the dbg_values which have not already been inserted in source
+  // Insert all the DBG_VALUEs which have not already been inserted in source
   // order sequence.
   if (HasDbg) {
-    MachineBasicBlock::iterator BBBegin = BB->getFirstNonPHI();
+    // Always place DBG_VALUEs after all other special start-of-block
+    // instructions, to avoid debug data hiding start-of-block instructions.
+    MachineBasicBlock::iterator BBBegin = BB->SkipPHIsAndLabels(BB->begin());
 
     // Sort the source order instructions and use the order to insert debug
     // values. Use stable_sort so that DBG_VALUEs are inserted in the same order

diff --git a/llvm/test/CodeGen/X86/dbg-changes-codegen-phi-elimination.ll b/llvm/test/CodeGen/X86/dbg-changes-codegen-phi-elimination.ll
new file mode 100644 (file)
index 0000000..0077673
--- /dev/null
+++ b/llvm/test/CodeGen/X86/dbg-changes-codegen-phi-elimination.ll
@@ -0,0 +1,66 @@
+; RUN: llc -O0 %s -mtriple=x86_64 -stop-after=phi-node-elimination -verify-machineinstrs -o - | FileCheck %s
+
+; Test that the DBG_VALUE must be after PHI and label, the PHI is converted to
+; a copy and lower after Label. The DBG_VALUE should not impact PHI lowering.
+; before:
+;    phi
+;    dbg_value
+;    label
+; after:
+;    LABEL
+;    COPY
+;    DBG_VALUE
+; Fix the issue: https://bugs.llvm.org/show_bug.cgi?id=43859
+
+define i32 @func(i32 %a0) personality void ()* @personality !dbg !9 {
+bb1:
+  %v = icmp slt i32 %a0, 0
+  br i1 %v, label %bb2, label %bb3
+
+bb2:
+  br label %bb3
+
+bb3:
+; CHECK: bb.2.bb3:
+; CHECK: EH_LABEL
+; CHECK-NEXT: %0:gr32 = COPY %7
+; CHECK-NEXT: DBG_VALUE %0
+  %0 = phi i32 [ 2, %bb2 ], [ 1, %bb1 ]
+  call void @llvm.dbg.value(metadata i32 %0, metadata !8, metadata !DIExpression()), !dbg !13
+  invoke void @foo() to label %end unwind label %lpad
+
+end:
+  ret i32 %0
+
+lpad:
+  %1 = landingpad { i8*, i32 }
+           cleanup
+  unreachable
+}
+
+declare void @foo()
+
+declare void @personality()
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "a.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 1, !"min_enum_size", i32 4}
+!7 = !{!"clang"}
+!8 = !DILocalVariable(name: "c", scope: !9, file: !1, line: 3, type: !12)
+!9 = distinct !DISubprogram(name: "func", scope: !1, file: !1, line: 1, type: !10, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!10 = !DISubroutineType(types: !11)
+!11 = !{!12, !12}
+!12 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!13 = !DILocation(line: 3, column: 13, scope: !9)