[llvm] 5491a86 - [DebugInfo] Emit DBG_VALUE_LIST from ISel

Stephen Tozer via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 9 04:18:04 PST 2021


Author: gbtozers
Date: 2021-03-09T12:17:39Z
New Revision: 5491a86f59ceff34b37635723fcba66cc3580c45

URL: https://github.com/llvm/llvm-project/commit/5491a86f59ceff34b37635723fcba66cc3580c45
DIFF: https://github.com/llvm/llvm-project/commit/5491a86f59ceff34b37635723fcba66cc3580c45.diff

LOG: [DebugInfo] Emit DBG_VALUE_LIST from ISel

This patch completes ISel support for DIArgList dbg.values by allowing
SDDbgValues with multiple location operands to be emitted as DBG_VALUE_LIST
instructions.

The primary change of this patch is refactoring EmitDbgValue by pulling location
operand emission out to the new function AddDbgValueLocationOps, which is used
for both DIArgList and single value dbg.values. Outside of that, the only
behaviour change is that the scheduler has a lambda added, HasUnknownVReg, to
prevent us from attempting to emit a DBG_VALUE_LIST before all of its used VRegs
have become available.

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

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
    llvm/lib/CodeGen/SelectionDAG/InstrEmitter.h
    llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
    llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp b/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
index c50f05079eee..84a92a02bb03 100644
--- a/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
@@ -684,6 +684,28 @@ InstrEmitter::EmitDbgValue(SDDbgValue *SD,
 
   SD->setIsEmitted();
 
+  ArrayRef<SDDbgOperand> LocationOps = SD->getLocationOps();
+  assert(!LocationOps.empty() && "dbg_value with no location operands?");
+
+  // Emit variadic dbg_value nodes as DBG_VALUE_LIST.
+  if (SD->isVariadic()) {
+    // DBG_VALUE_LIST := "DBG_VALUE_LIST" var, expression, loc (, loc)*
+    const MCInstrDesc &DbgValDesc = TII->get(TargetOpcode::DBG_VALUE_LIST);
+    // Build the DBG_VALUE_LIST instruction base.
+    auto MIB = BuildMI(*MF, DL, DbgValDesc);
+    MIB.addMetadata(Var);
+    MIB.addMetadata(Expr);
+    if (SD->isInvalidated()) {
+      // At least one node is no longer computed so we are unable to emit the
+      // location. Simply mark all of the operands as undef.
+      for (size_t i = 0; i < LocationOps.size(); ++i)
+        MIB.addReg(0U, RegState::Debug);
+    } else {
+      AddDbgValueLocationOps(MIB, DbgValDesc, LocationOps, VRBaseMap);
+    }
+    return &*MIB;
+  }
+
   if (SD->isInvalidated()) {
     // An invalidated SDNode must generate an undef DBG_VALUE: although the
     // original value is no longer computed, earlier DBG_VALUEs live ranges
@@ -697,77 +719,79 @@ InstrEmitter::EmitDbgValue(SDDbgValue *SD,
   }
 
   // Attempt to produce a DBG_INSTR_REF if we've been asked to.
+  // We currently exclude the possibility of instruction references for
+  // variadic nodes; if at some point we enable them, this should be moved
+  // above the variadic block.
   if (EmitDebugInstrRefs)
     if (auto *InstrRef = EmitDbgInstrRef(SD, VRBaseMap))
       return InstrRef;
 
-  SDDbgOperand SDOperand = SD->getLocationOps()[0];
-  if (SDOperand.getKind() == SDDbgOperand::FRAMEIX) {
-    // Stack address; this needs to be lowered in target-dependent fashion.
-    // EmitTargetCodeForFrameDebugValue is responsible for allocation.
-    auto FrameMI = BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE))
-                       .addFrameIndex(SDOperand.getFrameIx());
-    if (SD->isIndirect())
-      // Push [fi + 0] onto the DIExpression stack.
-      FrameMI.addImm(0);
-    else
-      // Push fi onto the DIExpression stack.
-      FrameMI.addReg(0);
-    return FrameMI.addMetadata(Var).addMetadata(Expr);
-  }
-  // Otherwise, we're going to create an instruction here.
-  const MCInstrDesc &II = TII->get(TargetOpcode::DBG_VALUE);
-  MachineInstrBuilder MIB = BuildMI(*MF, DL, II);
-  if (SDOperand.getKind() == SDDbgOperand::SDNODE) {
-    SDNode *Node = SDOperand.getSDNode();
-    SDValue Op = SDValue(Node, SDOperand.getResNo());
-    // It's possible we replaced this SDNode with other(s) and therefore
-    // didn't generate code for it.  It's better to catch these cases where
-    // they happen and transfer the debug info, but trying to guarantee that
-    // in all cases would be very fragile; this is a safeguard for any
-    // that were missed.
-    DenseMap<SDValue, Register>::iterator I = VRBaseMap.find(Op);
-    if (I==VRBaseMap.end())
-      MIB.addReg(0U);       // undef
-    else
-      AddOperand(MIB, Op, (*MIB).getNumOperands(), &II, VRBaseMap,
-                 /*IsDebug=*/true, /*IsClone=*/false, /*IsCloned=*/false);
-  } else if (SDOperand.getKind() == SDDbgOperand::VREG) {
-    MIB.addReg(SDOperand.getVReg(), RegState::Debug);
-  } else if (SDOperand.getKind() == SDDbgOperand::CONST) {
-    const Value *V = SDOperand.getConst();
-    if (const ConstantInt *CI = dyn_cast<ConstantInt>(V)) {
-      if (CI->getBitWidth() > 64)
-        MIB.addCImm(CI);
-      else
-        MIB.addImm(CI->getSExtValue());
-    } else if (const ConstantFP *CF = dyn_cast<ConstantFP>(V)) {
-      MIB.addFPImm(CF);
-    } else if (isa<ConstantPointerNull>(V)) {
-      // Note: This assumes that all nullptr constants are zero-valued.
-      MIB.addImm(0);
-    } else {
-      // Could be an Undef.  In any case insert an Undef so we can see what we
-      // dropped.
-      MIB.addReg(0U);
-    }
-  } else {
-    // Insert an Undef so we can see what we dropped.
-    MIB.addReg(0U);
-  }
-
-  // Indirect addressing is indicated by an Imm as the second parameter.
+  // Emit non-variadic dbg_value nodes as DBG_VALUE.
+  // DBG_VALUE := "DBG_VALUE" loc, isIndirect, var, expr
+  const MCInstrDesc &DbgValDesc = TII->get(TargetOpcode::DBG_VALUE);
+  // Build the DBG_VALUE instruction base.
+  auto MIB = BuildMI(*MF, DL, DbgValDesc);
+  // Add the single location componenet 'loc'.
+  assert(LocationOps.size() == 1 &&
+         "Non variadic dbg_value should have only one location op");
+  AddDbgValueLocationOps(MIB, DbgValDesc, LocationOps, VRBaseMap);
+  // Add 'isIndirect'.
   if (SD->isIndirect())
     MIB.addImm(0U);
   else
     MIB.addReg(0U, RegState::Debug);
-
   MIB.addMetadata(Var);
   MIB.addMetadata(Expr);
-
   return &*MIB;
 }
 
+void InstrEmitter::AddDbgValueLocationOps(
+    MachineInstrBuilder &MIB, const MCInstrDesc &DbgValDesc,
+    ArrayRef<SDDbgOperand> LocationOps,
+    DenseMap<SDValue, Register> &VRBaseMap) {
+  for (const SDDbgOperand &Op : LocationOps) {
+    switch (Op.getKind()) {
+    case SDDbgOperand::FRAMEIX:
+      MIB.addFrameIndex(Op.getFrameIx());
+      break;
+    case SDDbgOperand::VREG:
+      MIB.addReg(Op.getVReg(), RegState::Debug);
+      break;
+    case SDDbgOperand::SDNODE: {
+      SDValue V = SDValue(Op.getSDNode(), Op.getResNo());
+      // It's possible we replaced this SDNode with other(s) and therefore
+      // didn't generate code for it. It's better to catch these cases where
+      // they happen and transfer the debug info, but trying to guarantee that
+      // in all cases would be very fragile; this is a safeguard for any
+      // that were missed.
+      if (VRBaseMap.count(V) == 0)
+        MIB.addReg(0U); // undef
+      else
+        AddOperand(MIB, V, (*MIB).getNumOperands(), &DbgValDesc, VRBaseMap,
+                   /*IsDebug=*/true, /*IsClone=*/false, /*IsCloned=*/false);
+    } break;
+    case SDDbgOperand::CONST: {
+      const Value *V = Op.getConst();
+      if (const ConstantInt *CI = dyn_cast<ConstantInt>(V)) {
+        if (CI->getBitWidth() > 64)
+          MIB.addCImm(CI);
+        else
+          MIB.addImm(CI->getSExtValue());
+      } else if (const ConstantFP *CF = dyn_cast<ConstantFP>(V)) {
+        MIB.addFPImm(CF);
+      } else if (isa<ConstantPointerNull>(V)) {
+        // Note: This assumes that all nullptr constants are zero-valued.
+        MIB.addImm(0);
+      } else {
+        // Could be an Undef. In any case insert an Undef so we can see what we
+        // dropped.
+        MIB.addReg(0U);
+      }
+    } break;
+    }
+  }
+}
+
 MachineInstr *
 InstrEmitter::EmitDbgInstrRef(SDDbgValue *SD,
                               DenseMap<SDValue, Register> &VRBaseMap) {

diff  --git a/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.h b/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.h
index 09658b8143fe..7bf0e0c4fb19 100644
--- a/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.h
+++ b/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.h
@@ -25,6 +25,7 @@ class MachineInstrBuilder;
 class MCInstrDesc;
 class SDDbgLabel;
 class SDDbgValue;
+class SDDbgOperand;
 class TargetLowering;
 class TargetMachine;
 
@@ -108,6 +109,11 @@ class LLVM_LIBRARY_VISIBILITY InstrEmitter {
   /// (which do not go into the machine instrs.)
   static unsigned CountResults(SDNode *Node);
 
+  void AddDbgValueLocationOps(MachineInstrBuilder &MIB,
+                              const MCInstrDesc &DbgValDesc,
+                              ArrayRef<SDDbgOperand> Locations,
+                              DenseMap<SDValue, Register> &VRBaseMap);
+
   /// EmitDbgValue - Generate machine instruction for a dbg_value node.
   ///
   MachineInstr *EmitDbgValue(SDDbgValue *SD,
@@ -148,7 +154,6 @@ class LLVM_LIBRARY_VISIBILITY InstrEmitter {
   void EmitSpecialNode(SDNode *Node, bool IsClone, bool IsCloned,
                        DenseMap<SDValue, Register> &VRBaseMap);
 };
-
-}
+} // namespace llvm
 
 #endif

diff  --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
index 277a32283b0d..dd02ac34326e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
@@ -739,6 +739,17 @@ ProcessSDDbgValues(SDNode *N, SelectionDAG *DAG, InstrEmitter &Emitter,
   if (!N->getHasDebugValue())
     return;
 
+  /// Returns true if \p DV has any VReg operand locations which don't exist in
+  /// VRBaseMap.
+  auto HasUnknownVReg = [&VRBaseMap](SDDbgValue *DV) {
+    for (SDDbgOperand L : DV->getLocationOps()) {
+      if (L.getKind() == SDDbgOperand::SDNODE &&
+          VRBaseMap.count({L.getSDNode(), L.getResNo()}) == 0)
+        return true;
+    }
+    return false;
+  };
+
   // Opportunistically insert immediate dbg_value uses, i.e. those with the same
   // source order number as N.
   MachineBasicBlock *BB = Emitter.getBlock();
@@ -747,13 +758,20 @@ ProcessSDDbgValues(SDNode *N, SelectionDAG *DAG, InstrEmitter &Emitter,
     if (DV->isEmitted())
       continue;
     unsigned DVOrder = DV->getOrder();
-    if (!Order || DVOrder == Order) {
-      MachineInstr *DbgMI = Emitter.EmitDbgValue(DV, VRBaseMap);
-      if (DbgMI) {
-        Orders.push_back({DVOrder, DbgMI});
-        BB->insert(InsertPos, DbgMI);
-      }
-    }
+    if (Order != 0 && DVOrder != Order)
+      continue;
+    // If DV has any VReg location operands which haven't been mapped then
+    // either that node is no longer available or we just haven't visited the
+    // node yet. In the former case we should emit an undef dbg_value, but we
+    // can do it later. And for the latter we'll want to wait until all
+    // dependent nodes have been visited.
+    if (HasUnknownVReg(DV))
+      continue;
+    MachineInstr *DbgMI = Emitter.EmitDbgValue(DV, VRBaseMap);
+    if (!DbgMI)
+      continue;
+    Orders.push_back({DVOrder, DbgMI});
+    BB->insert(InsertPos, DbgMI);
   }
 }
 

diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 19920077e682..64e74d936243 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -572,7 +572,9 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
 
   // Insert DBG_VALUE instructions for function arguments to the entry block.
   for (unsigned i = 0, e = FuncInfo->ArgDbgValues.size(); i != e; ++i) {
-    MachineInstr *MI = FuncInfo->ArgDbgValues[e-i-1];
+    MachineInstr *MI = FuncInfo->ArgDbgValues[e - i - 1];
+    assert(MI->getOpcode() != TargetOpcode::DBG_VALUE_LIST &&
+           "Function parameters should not be described by DBG_VALUE_LIST.");
     bool hasFI = MI->getOperand(0).isFI();
     Register Reg =
         hasFI ? TRI.getFrameRegister(*MF) : MI->getOperand(0).getReg();
@@ -605,6 +607,8 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
                "DBG_VALUE with nonzero offset");
       assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
              "Expected inlined-at fields to agree");
+      assert(MI->getOpcode() != TargetOpcode::DBG_VALUE_LIST &&
+             "Didn't expect to see a DBG_VALUE_LIST here");
       // Def is never a terminator here, so it is ok to increment InsertPos.
       BuildMI(*EntryMBB, ++InsertPos, DL, TII->get(TargetOpcode::DBG_VALUE),
               IsIndirect, LDI->second, Variable, Expr);


        


More information about the llvm-commits mailing list