[DebugInfo] Make sure all DBG_VALUEs' reguse operands have IsDebug property

Summary: In some cases, these operands lacked the IsDebug property, which is meant to signal that they should not affect codegen. This patch adds a check for this property in the MachineVerifier and adds it where it was missing. This includes refactorings to use MachineInstrBuilder construction functions instead of manually setting up the intrinsic everywhere. Patch by: JesperAntonsson Reviewers: aprantl, rnk, echristo, javed.absar Reviewed By: aprantl Subscribers: qcolombet, sdardis, nemanjai, JDevlieghere, atanasyan, llvm-commits Differential Revision: https://reviews.llvm.org/D48319 llvm-svn: 335214
2024-11-22 10:42:39 +01:00 · 2018-06-21 10:03:34 +00:00 · 2018-06-21 10:03:34 +00:00 · 96b4fab48a
commit 96b4fab48a
parent 281ed964df
16 changed files with 82 additions and 62 deletions
--- a/include/llvm/CodeGen/MachineInstrBuilder.h
+++ b/include/llvm/CodeGen/MachineInstrBuilder.h
@ -417,6 +417,13 @@ MachineInstrBuilder BuildMI(MachineFunction &MF, const DebugLoc &DL,
                            unsigned Reg, const MDNode *Variable,
                            const MDNode *Expr);

+/// This version of the builder builds a DBG_VALUE intrinsic
+/// for a MachineOperand.
+MachineInstrBuilder BuildMI(MachineFunction &MF, const DebugLoc &DL,
+                            const MCInstrDesc &MCID, bool IsIndirect,
+                            MachineOperand &MO, const MDNode *Variable,
+                            const MDNode *Expr);
+
 /// This version of the builder builds a DBG_VALUE intrinsic
 /// for either a value in a register or a register-indirect
 /// address and inserts it at position I.
@ -426,6 +433,14 @@ MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
                            unsigned Reg, const MDNode *Variable,
                            const MDNode *Expr);

+/// This version of the builder builds a DBG_VALUE intrinsic
+/// for a machine operand and inserts it at position I.
+MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
+                            MachineBasicBlock::iterator I, const DebugLoc &DL,
+                            const MCInstrDesc &MCID, bool IsIndirect,
+                            MachineOperand &MO, const MDNode *Variable,
+                            const MDNode *Expr);
+
 /// Clone a DBG_VALUE whose value has been spilled to FrameIndex.
 MachineInstr *buildDbgValueForSpill(MachineBasicBlock &BB,
                                    MachineBasicBlock::iterator I,
--- a/lib/CodeGen/LiveDebugVariables.cpp
+++ b/lib/CodeGen/LiveDebugVariables.cpp
@ -1196,14 +1196,8 @@ void UserValue::insertDebugValue(MachineBasicBlock *MBB, SlotIndex StartIdx,
  assert((!Spilled || MO.isFI()) && "a spilled location must be a frame index");

  do {
-    MachineInstrBuilder MIB =
-      BuildMI(*MBB, I, getDebugLoc(), TII.get(TargetOpcode::DBG_VALUE))
-          .add(MO);
-    if (IsIndirect)
-      MIB.addImm(0U);
-    else
-      MIB.addReg(0U, RegState::Debug);
-    MIB.addMetadata(Variable).addMetadata(Expr);
+    BuildMI(*MBB, I, getDebugLoc(), TII.get(TargetOpcode::DBG_VALUE),
+            IsIndirect, MO, Variable, Expr);

    // Continue and insert DBG_VALUES after every redefinition of register
    // associated with the debug value within the range
--- a/lib/CodeGen/MachineInstr.cpp
+++ b/lib/CodeGen/MachineInstr.cpp
@ -1794,33 +1794,55 @@ MachineInstrBuilder llvm::BuildMI(MachineFunction &MF, const DebugLoc &DL,
  assert(cast<DIExpression>(Expr)->isValid() && "not an expression");
  assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
         "Expected inlined-at fields to agree");
+  auto MIB = BuildMI(MF, DL, MCID).addReg(Reg, RegState::Debug);
  if (IsIndirect)
-    return BuildMI(MF, DL, MCID)
-        .addReg(Reg, RegState::Debug)
-        .addImm(0U)
-        .addMetadata(Variable)
-        .addMetadata(Expr);
+    MIB.addImm(0U);
  else
-    return BuildMI(MF, DL, MCID)
-        .addReg(Reg, RegState::Debug)
-        .addReg(0U, RegState::Debug)
-        .addMetadata(Variable)
-        .addMetadata(Expr);
+    MIB.addReg(0U, RegState::Debug);
+  return MIB.addMetadata(Variable).addMetadata(Expr);
 }

+MachineInstrBuilder llvm::BuildMI(MachineFunction &MF, const DebugLoc &DL,
+                                  const MCInstrDesc &MCID, bool IsIndirect,
+                                  MachineOperand &MO, const MDNode *Variable,
+                                  const MDNode *Expr) {
+  assert(isa<DILocalVariable>(Variable) && "not a variable");
+  assert(cast<DIExpression>(Expr)->isValid() && "not an expression");
+  assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
+         "Expected inlined-at fields to agree");
+  if (MO.isReg())
+    return BuildMI(MF, DL, MCID, IsIndirect, MO.getReg(), Variable, Expr);
+
+  auto MIB = BuildMI(MF, DL, MCID).add(MO);
+  if (IsIndirect)
+    MIB.addImm(0U);
+  else
+    MIB.addReg(0U, RegState::Debug);
+  return MIB.addMetadata(Variable).addMetadata(Expr);
+ }
+
 MachineInstrBuilder llvm::BuildMI(MachineBasicBlock &BB,
                                  MachineBasicBlock::iterator I,
                                  const DebugLoc &DL, const MCInstrDesc &MCID,
                                  bool IsIndirect, unsigned Reg,
                                  const MDNode *Variable, const MDNode *Expr) {
-  assert(isa<DILocalVariable>(Variable) && "not a variable");
-  assert(cast<DIExpression>(Expr)->isValid() && "not an expression");
  MachineFunction &MF = *BB.getParent();
  MachineInstr *MI = BuildMI(MF, DL, MCID, IsIndirect, Reg, Variable, Expr);
  BB.insert(I, MI);
  return MachineInstrBuilder(MF, MI);
 }

+MachineInstrBuilder llvm::BuildMI(MachineBasicBlock &BB,
+                                  MachineBasicBlock::iterator I,
+                                  const DebugLoc &DL, const MCInstrDesc &MCID,
+                                  bool IsIndirect, MachineOperand &MO,
+                                  const MDNode *Variable, const MDNode *Expr) {
+  MachineFunction &MF = *BB.getParent();
+  MachineInstr *MI = BuildMI(MF, DL, MCID, IsIndirect, MO, Variable, Expr);
+  BB.insert(I, MI);
+  return MachineInstrBuilder(MF, *MI);
+}
+
 /// Compute the new DIExpression to use with a DBG_VALUE for a spill slot.
 /// This prepends DW_OP_deref when spilling an indirect DBG_VALUE.
 static const DIExpression *computeExprForSpill(const MachineInstr &MI) {
--- a/lib/CodeGen/MachineVerifier.cpp
+++ b/lib/CodeGen/MachineVerifier.cpp
@ -1204,6 +1204,10 @@ MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
          return;
        }
      }
+      if (MI->isDebugValue() && MO->isUse() && !MO->isDebug()) {
+        report("Use-reg is not IsDebug in a DBG_VALUE", MO, MONum);
+        return;
+      }
    } else {
      // Virtual register.
      const TargetRegisterClass *RC = MRI->getRegClassOrNull(Reg);
--- a/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/lib/CodeGen/PrologEpilogInserter.cpp
@ -1074,6 +1074,7 @@ void PEI::replaceFrameIndices(MachineBasicBlock *BB, MachineFunction &MF,
        int64_t Offset =
            TFI->getFrameIndexReference(MF, MI.getOperand(0).getIndex(), Reg);
        MI.getOperand(0).ChangeToRegister(Reg, false /*isDef*/);
+        MI.getOperand(0).setIsDebug();
        auto *DIExpr = DIExpression::prepend(MI.getDebugExpression(),
                                             DIExpression::NoDeref, Offset);
        MI.getOperand(3).setMetadata(DIExpr);
--- a/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/lib/CodeGen/SelectionDAG/FastISel.cpp
@ -1371,20 +1371,11 @@ bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) {
    if (Op) {
      assert(DI->getVariable()->isValidLocationForIntrinsic(DbgLoc) &&
             "Expected inlined-at fields to agree");
-      if (Op->isReg()) {
-        Op->setIsDebug(true);
-        // A dbg.declare describes the address of a source variable, so lower it
-        // into an indirect DBG_VALUE.
-        BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
-                TII.get(TargetOpcode::DBG_VALUE), /*IsIndirect*/ true,
-                Op->getReg(), DI->getVariable(), DI->getExpression());
-      } else
-        BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
-                TII.get(TargetOpcode::DBG_VALUE))
-            .add(*Op)
-            .addImm(0)
-            .addMetadata(DI->getVariable())
-            .addMetadata(DI->getExpression());
+      // A dbg.declare describes the address of a source variable, so lower it
+      // into an indirect DBG_VALUE.
+      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
+              TII.get(TargetOpcode::DBG_VALUE), /*IsIndirect*/ true,
+              *Op, DI->getVariable(), DI->getExpression());
    } else {
      // We can't yet handle anything else here because it would require
      // generating code, thus altering codegen because of debug info.
--- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@ -4951,17 +4951,10 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(

  assert(Variable->isValidLocationForIntrinsic(DL) &&
         "Expected inlined-at fields to agree");
-  if (Op->isReg())
-    FuncInfo.ArgDbgValues.push_back(
-        BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,
-                Op->getReg(), Variable, Expr));
-  else
-    FuncInfo.ArgDbgValues.push_back(
-        BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE))
-            .add(*Op)
-            .addImm(0)
-            .addMetadata(Variable)
-            .addMetadata(Expr));
+  IsIndirect = (Op->isReg()) ? IsIndirect : true;
+  FuncInfo.ArgDbgValues.push_back(
+      BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,
+              *Op, Variable, Expr));

  return true;
 }
--- a/test/CodeGen/MIR/X86/diexpr-win32.mir
+++ b/test/CodeGen/MIR/X86/diexpr-win32.mir
@ -194,8 +194,8 @@ body:             |
    CFI_INSTRUCTION def_cfa_offset 8
    CFI_INSTRUCTION offset $esi, -8
    $esi = MOV32rm $esp, 1, _, 8, _ :: (load 4 from %fixed-stack.2)
-    DBG_VALUE $esp, 0, !26, !10, debug-location !25
-    DBG_VALUE $esp, 0, !23, !DIExpression(DW_OP_plus_uconst, 8, DW_OP_deref), debug-location !25
+    DBG_VALUE debug-use $esp, 0, !26, !10, debug-location !25
+    DBG_VALUE debug-use $esp, 0, !23, !DIExpression(DW_OP_plus_uconst, 8, DW_OP_deref), debug-location !25
    CALLpcrel32 @getString, csr_32, implicit $esp, implicit-def $esp, implicit-def $eax, debug-location !29
    $ecx = MOV32rm $eax, 1, _, 0, _, debug-location !29 :: (dereferenceable load 4 from %ir.1)
    $edx = MOV32rm $eax, 1, _, 4, _, debug-location !29 :: (dereferenceable load 4 from %ir.1 + 4)
--- a/test/CodeGen/PowerPC/debuginfo-stackarg.ll
+++ b/test/CodeGen/PowerPC/debuginfo-stackarg.ll
@ -33,7 +33,7 @@ define i64 @foo(i64 %bar1, i64 %bar2, i64 %bar3, i64 %bar4, i64 %bar5) local_unn
 ; We expect to find a DBG_VALUE refering to the metadata id for bar5, using the lowest
 ; of the two fixed stack offsets found earlier.
 ; CHECK-LABEL: body:
-; CHECK: DBG_VALUE $r1, 0, !17, !DIExpression(DW_OP_plus_uconst, 56)
+; CHECK: DBG_VALUE debug-use $r1, 0, !17, !DIExpression(DW_OP_plus_uconst, 56)
 entry:
  tail call void @llvm.dbg.value(metadata i64 %bar1, metadata !13, metadata !DIExpression()), !dbg !18
  tail call void @llvm.dbg.value(metadata i64 %bar2, metadata !14, metadata !DIExpression()), !dbg !19
--- a/test/CodeGen/X86/post-ra-sched-with-debug.mir
+++ b/test/CodeGen/X86/post-ra-sched-with-debug.mir
@ -288,7 +288,7 @@ body:             |
    $rcx = LEA64r $rbp, 1, $noreg, -20, $noreg
    DBG_VALUE debug-use $rcx, debug-use $noreg, !46, !17, debug-location !48
    DBG_VALUE debug-use $rcx, debug-use $noreg, !39, !17, debug-location !44
-    DBG_VALUE $rbp, -20, !29, !17, debug-location !36
+    DBG_VALUE debug-use $rbp, -20, !29, !17, debug-location !36
    $rcx = CMOVNE64rr killed $rcx, killed $rdx, implicit killed $eflags
    $rcx = OR64rr killed $rcx, killed $rsi, implicit-def dead $eflags
    $rdx = MOVSX64rm32 $rbx, 1, $noreg, 0, $noreg :: (load 4, align 8)
--- a/test/DebugInfo/MIR/AArch64/clobber-sp.mir
+++ b/test/DebugInfo/MIR/AArch64/clobber-sp.mir
@ -149,7 +149,7 @@ body:             |
    STURWi killed $w0, $fp, -4 :: (store 4 into %stack.0.x.addr)
    DBG_VALUE debug-use $w1, debug-use _, !20, !22, debug-location !28
    STRWui killed $w1, $sp, 2, debug-location !30 :: (store 4 into %stack.1)
-    DBG_VALUE $sp, 0, !20, !36, debug-location !28
+    DBG_VALUE debug-use $sp, 0, !20, !36, debug-location !28
    BL @g, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $d0, implicit killed $d1, implicit killed $d2, implicit killed $d3, implicit-def $sp, debug-location !30
    $w0 = LDRWui $sp, 2, debug-location !33 :: (load 4 from %stack.1)
    CBZW killed $w0, %bb.2.if.end, debug-location !33
--- a/test/DebugInfo/MIR/Mips/last-inst-bundled.mir
+++ b/test/DebugInfo/MIR/Mips/last-inst-bundled.mir
@ -164,7 +164,7 @@ body:             |
    DBG_VALUE debug-use $a0, debug-use $noreg, !12, !DIExpression(), debug-location !17
    $s0 = OR $a0, $zero
    DBG_VALUE debug-use $s0, debug-use $noreg, !12, !DIExpression(), debug-location !17
-    DBG_VALUE $sp, 0, !13, !DIExpression(DW_OP_plus_uconst, 20), debug-location !19
+    DBG_VALUE debug-use $sp, 0, !13, !DIExpression(DW_OP_plus_uconst, 20), debug-location !19
    JAL @set_cond, csr_o32, implicit-def dead $ra, implicit $a0, implicit $a1, implicit-def $sp, debug-location !20 {
      renamable $a1 = LEA_ADDiu $sp, 20
    }
--- a/test/DebugInfo/MIR/X86/kill-after-spill.mir
+++ b/test/DebugInfo/MIR/X86/kill-after-spill.mir
@ -277,9 +277,9 @@ body:             |
    DBG_VALUE debug-use $edi, debug-use $noreg, !36, !DIExpression(), debug-location !57
    DBG_VALUE debug-use $esi, debug-use $noreg, !37, !DIExpression(), debug-location !58
    $ebx = MOV32rr $esi
-    DBG_VALUE $ebx, debug-use $noreg, !37, !DIExpression(), debug-location !58
+    DBG_VALUE debug-use $ebx, debug-use $noreg, !37, !DIExpression(), debug-location !58
    $r15d = MOV32rr $edi
-    DBG_VALUE $r15d, debug-use $noreg, !36, !DIExpression(), debug-location !57
+    DBG_VALUE debug-use $r15d, debug-use $noreg, !36, !DIExpression(), debug-location !57
    renamable $r14 = MOV64ri -9223372036854775808
    $edi = MOV32rr $ebx
    CALL64pcrel32 @func1, csr_64, implicit $rsp, implicit $ssp, implicit $edi, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
--- a/test/DebugInfo/MIR/X86/live-debug-values-spill.mir
+++ b/test/DebugInfo/MIR/X86/live-debug-values-spill.mir
@ -421,11 +421,11 @@ body:             |
    liveins: $r14d, $r15d, $rbp
  
    $rdi = LEA64r $rbp, 1, _, -44, _
-    DBG_VALUE $rbp, -44, !35, !38, debug-location !60
+    DBG_VALUE debug-use $rbp, -44, !35, !38, debug-location !60
    $rsi = LEA64r $rbp, 1, _, -60, _
-    DBG_VALUE $rbp, -60, !36, !38, debug-location !63
+    DBG_VALUE debug-use $rbp, -60, !36, !38, debug-location !63
    $rdx = LEA64r $rbp, 1, _, -64, _
-    DBG_VALUE $rbp, -64, !37, !38, debug-location !78
+    DBG_VALUE debug-use $rbp, -64, !37, !38, debug-location !78
    CALL64pcrel32 @set, csr_64, implicit $rsp, implicit $rdi, implicit $rsi, implicit $rdx, implicit-def $rsp, debug-location !79
    $eax = MOV32rm $rbp, 1, _, -44, _, debug-location !81 :: (dereferenceable load 4 from %ir.inte, !tbaa !47)
    DBG_VALUE debug-use $eax, debug-use _, !35, !38, debug-location !60
--- a/test/DebugInfo/X86/bbjoin.ll
+++ b/test/DebugInfo/X86/bbjoin.ll
@ -12,7 +12,7 @@
 ; CHECK: ![[X:.*]] = !DILocalVariable(name: "x",
 ; CHECK: bb.0.entry:
 ; CHECK:   DBG_VALUE 23, debug-use $noreg, ![[X]],
-; CHECK:   DBG_VALUE $rsp, 0, ![[X]], !DIExpression(DW_OP_plus_uconst, 4, DW_OP_deref),
+; CHECK:   DBG_VALUE debug-use $rsp, 0, ![[X]], !DIExpression(DW_OP_plus_uconst, 4, DW_OP_deref),
 ; CHECK: bb.1.if.then:
 ; CHECK:   DBG_VALUE 43, debug-use $noreg, ![[X]],
 ; CHECK: bb.2.if.end:
--- a/test/DebugInfo/X86/pr34545.ll
+++ b/test/DebugInfo/X86/pr34545.ll
@ -1,13 +1,13 @@
 ; RUN: llc -O1 -filetype=asm -mtriple x86_64-unknown-linux-gnu -mcpu=x86-64 -o - %s -stop-after=livedebugvars | FileCheck %s

 ; CHECK: $eax = MOV32rm
-; CHECK: DBG_VALUE $eax
+; CHECK: DBG_VALUE debug-use $eax
 ; CHECK: $eax = SHL32rCL killed renamable $eax
-; CHECK: DBG_VALUE $eax
-; CHECK: DBG_VALUE $rsp, 0, !{{[0-9]+}}, !DIExpression(DW_OP_constu, 4, DW_OP_minus)
-; CHECK: DBG_VALUE $eax
+; CHECK: DBG_VALUE debug-use $eax
+; CHECK: DBG_VALUE debug-use $rsp, 0, !{{[0-9]+}}, !DIExpression(DW_OP_constu, 4, DW_OP_minus)
+; CHECK: DBG_VALUE debug-use $eax
 ; CHECK: $eax = SHL32rCL killed renamable $eax
-; CHECK: DBG_VALUE $eax
+; CHECK: DBG_VALUE debug-use $eax
 ; CHECK: RETQ $eax

 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"