[DebugInfo] Allow non-stack_value variadic expressions and use in DBG_INSTR_REF

Prior to this patch, variadic DIExpressions (i.e. ones that contain
DW_OP_LLVM_arg) could only be created by salvaging debug values to create
stack value expressions, resulting in a DBG_VALUE_LIST being created. As of
the previous patch in this patch stack, DBG_INSTR_REF's syntax has been
changed to match DBG_VALUE_LIST in preparation for supporting variadic
expressions. This patch adds some minor changes needed to allow variadic
expressions that aren't stack values to exist, and allows variadic expressions
that are trivially reduceable to non-variadic expressions to be handled
similarly to non-variadic expressions.

Reviewed by: jmorse

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

Author: SLTozer

llvm/docs/MIRLangRef.rst

@@ -747,7 +747,7 @@ The example below uses a reference to Instruction 1, Operand 0:
 
 .. code-block:: text
 
-    DBG_INSTR_REF !123, !DIExpression(), dbg-instr-ref(1, 0), debug-location !456
+    DBG_INSTR_REF !123, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 0), debug-location !456
 
 CFIIndex Operands
 ^^^^^^^^^^^^^^^^^
@@ -913,7 +913,7 @@ instruction number and operand number. Consider the example below:
 .. code-block:: text
 
     $rbp = MOV64ri 0, debug-instr-number 1, debug-location !12
-    DBG_INSTR_REF !123, !DIExpression(), dbg-instr-ref(1, 0), debug-location !456
+    DBG_INSTR_REF !123, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 0), debug-location !456
 
 Instruction numbers are directly attached to machine instructions with an
 optional ``debug-instr-number`` attachment, before the optional

llvm/include/llvm/IR/DebugInfoMetadata.h

@@ -2794,6 +2794,17 @@ class DIExpression : public MDNode {
   /// it cannot be a simple register location.
   bool isComplex() const;
 
+  /// Return whether the evaluated expression makes use of a single location at
+  /// the start of the expression, i.e. if it contains only a single
+  /// DW_OP_LLVM_arg op as its first operand, or if it contains none.
+  bool isSingleLocationExpression() const;
+
+  /// If \p Expr is a non-variadic expression (i.e. one that does not contain
+  /// DW_OP_LLVM_arg), returns \p Expr converted to variadic form by adding a
+  /// leading [DW_OP_LLVM_arg, 0] to the expression; otherwise returns \p Expr.
+  static const DIExpression *
+  convertToVariadicExpression(const DIExpression *Expr);
+
   /// Inserts the elements of \p Expr into \p Ops modified to a canonical form,
   /// which uses DW_OP_LLVM_arg (i.e. is a variadic expression) and folds the
   /// implied derefence from the \p IsIndirect flag into the expression. This

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

@@ -1132,10 +1132,16 @@ static bool emitDebugValueComment(const MachineInstr *MI, AsmPrinter &AP) {
   OS << " <- ";
 
   const DIExpression *Expr = MI->getDebugExpression();
+  if (Expr->getNumElements() && Expr->isSingleLocationExpression() &&
+      Expr->expr_op_begin()->getOp() == dwarf::DW_OP_LLVM_arg) {
+    SmallVector<uint64_t> Ops(
+        make_range(Expr->elements_begin() + 2, Expr->elements_end()));
+    Expr = DIExpression::get(Expr->getContext(), Ops);
+  }
   if (Expr->getNumElements()) {
     OS << '[';
     ListSeparator LS;
-    for (auto Op : Expr->expr_ops()) {
+    for (auto &Op : Expr->expr_ops()) {
       OS << LS << dwarf::OperationEncodingString(Op.getOp());
       for (unsigned I = 0; I < Op.getNumArgs(); ++I)
         OS << ' ' << Op.getArg(I);

llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h

@@ -119,13 +119,7 @@ class DbgValueLoc {
 public:
   DbgValueLoc(const DIExpression *Expr, ArrayRef<DbgValueLocEntry> Locs)
       : Expression(Expr), ValueLocEntries(Locs.begin(), Locs.end()),
-        IsVariadic(true) {
-#ifndef NDEBUG
-    // Currently, DBG_VALUE_VAR expressions must use stack_value.
-    assert(Expr && Expr->isValid() &&
-           is_contained(Locs, dwarf::DW_OP_stack_value));
-#endif
-  }
+        IsVariadic(true) {}
 
   DbgValueLoc(const DIExpression *Expr, ArrayRef<DbgValueLocEntry> Locs,
               bool IsVariadic)
@@ -136,10 +130,6 @@ class DbgValueLoc {
            !any_of(Locs, [](auto LE) { return LE.isLocation(); }));
     if (!IsVariadic) {
       assert(ValueLocEntries.size() == 1);
-    } else {
-      // Currently, DBG_VALUE_VAR expressions must use stack_value.
-      assert(Expr && Expr->isValid() &&
-             is_contained(Expr->getElements(), dwarf::DW_OP_stack_value));
     }
 #endif
   }

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

@@ -1262,7 +1262,7 @@ MLocTracker::emitLoc(const SmallVectorImpl<ResolvedDbgOp> &DbgOps,
           // the pointer to the variable loaded off the stack with a deref:
           assert(!Expr->isImplicit());
           OffsetOps.push_back(dwarf::DW_OP_deref);
-        } else if (UseDerefSize && !Properties.IsVariadic) {
+        } else if (UseDerefSize && Expr->isSingleLocationExpression()) {
           // TODO: Figure out how to handle deref size issues for variadic
           // values.
           // We're loading a value off the stack that's not the same size as the
@@ -1271,7 +1271,7 @@ MLocTracker::emitLoc(const SmallVectorImpl<ResolvedDbgOp> &DbgOps,
           OffsetOps.push_back(dwarf::DW_OP_deref_size);
           OffsetOps.push_back(DerefSizeInBytes);
           StackValue = true;
-        } else if (Expr->isComplex()) {
+        } else if (Expr->isComplex() || Properties.IsVariadic) {
           // A variable with no size ambiguity, but with extra elements in it's
           // expression. Manually dereference the stack location.
           OffsetOps.push_back(dwarf::DW_OP_deref);
@@ -1590,7 +1590,7 @@ bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI,
   // about it. The rest of this LiveDebugValues implementation acts exactly the
   // same for DBG_INSTR_REFs as DBG_VALUEs (just, the former can refer to values
   // that aren't immediately available).
-  DbgValueProperties Properties(Expr, false, false);
+  DbgValueProperties Properties(Expr, false, true);
   SmallVector<DbgOpID> DbgOpIDs;
   if (NewID)
     DbgOpIDs.push_back(DbgOpStore.insert(*NewID));
@@ -1634,7 +1634,7 @@ bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI,
       NewID->getInst() > CurInst) {
     SmallVector<DbgOp> UseBeforeDefLocs;
     UseBeforeDefLocs.push_back(*NewID);
-    TTracker->addUseBeforeDef(V, {MI.getDebugExpression(), false, false},
+    TTracker->addUseBeforeDef(V, {MI.getDebugExpression(), false, true},
                               UseBeforeDefLocs, NewID->getInst());
   }
 

llvm/lib/CodeGen/LiveDebugVariables.cpp

@@ -138,8 +138,7 @@ class DbgVariableValue {
       // Turn this into an undef debug value list; right now, the simplest form
       // of this is an expression with one arg, and an undef debug operand.
       Expression =
-          DIExpression::get(Expr.getContext(), {dwarf::DW_OP_LLVM_arg, 0,
-                                                dwarf::DW_OP_stack_value});
+          DIExpression::get(Expr.getContext(), {dwarf::DW_OP_LLVM_arg, 0});
       if (auto FragmentInfoOpt = Expr.getFragmentInfo())
         Expression = *DIExpression::createFragmentExpression(
             Expression, FragmentInfoOpt->OffsetInBits,

llvm/lib/CodeGen/MachineFunction.cpp

@@ -44,6 +44,7 @@
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
@@ -1153,6 +1154,10 @@ void MachineFunction::finalizeDebugInstrRefs() {
         MakeUndefDbgValue(MI);
         continue;
       }
+      // Only convert Expr to variadic form when we're sure we're emitting a
+      // complete instruction reference.
+      MI.getDebugExpressionOp().setMetadata(
+          DIExpression::convertToVariadicExpression(MI.getDebugExpression()));
 
       assert(Reg.isVirtual());
       MachineInstr &DefMI = *RegInfo->def_instr_begin(Reg);

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

@@ -767,7 +767,7 @@ InstrEmitter::EmitDbgInstrRef(SDDbgValue *SD,
   assert(!SD->isVariadic());
   SDDbgOperand DbgOperand = SD->getLocationOps()[0];
   MDNode *Var = SD->getVariable();
-  DIExpression *Expr = (DIExpression*)SD->getExpression();
+  const DIExpression *Expr = (DIExpression *)SD->getExpression();
   DebugLoc DL = SD->getDebugLoc();
   const MCInstrDesc &RefII = TII->get(TargetOpcode::DBG_INSTR_REF);
 
@@ -851,6 +851,10 @@ InstrEmitter::EmitDbgInstrRef(SDDbgValue *SD,
   }
   assert(OperandIdx < DefMI->getNumOperands());
 
+  // Only convert Expr to variadic form when we're sure we're emitting a
+  // complete instruction reference.
+  if (!SD->isVariadic())
+    Expr = DIExpression::convertToVariadicExpression(Expr);
   // Make the DBG_INSTR_REF refer to that instruction, and that operand.
   unsigned InstrNum = DefMI->getDebugInstrNum();
   SmallVector<MachineOperand, 1> MOs(

llvm/lib/IR/DebugInfoMetadata.cpp

@@ -1358,12 +1358,15 @@ bool DIExpression::isValid() const {
       break;
     }
     case dwarf::DW_OP_LLVM_entry_value: {
-      // An entry value operator must appear at the beginning and the number of
-      // operations it cover can currently only be 1, because we support only
-      // entry values of a simple register location. One reason for this is that
-      // we currently can't calculate the size of the resulting DWARF block for
-      // other expressions.
-      return I->get() == expr_op_begin()->get() && I->getArg(0) == 1;
+      // An entry value operator must appear at the beginning or immediately
+      // following `DW_OP_LLVM_arg 0`, and the number of operations it cover can
+      // currently only be 1, because we support only entry values of a simple
+      // register location. One reason for this is that we currently can't
+      // calculate the size of the resulting DWARF block for other expressions.
+      auto FirstOp = expr_op_begin();
+      if (FirstOp->getOp() == dwarf::DW_OP_LLVM_arg && FirstOp->getArg(0) == 0)
+        ++FirstOp;
+      return I->get() == FirstOp->get() && I->getArg(0) == 1;
     }
     case dwarf::DW_OP_LLVM_implicit_pointer:
     case dwarf::DW_OP_LLVM_convert:
@@ -1432,6 +1435,7 @@ bool DIExpression::isComplex() const {
     switch (It.getOp()) {
     case dwarf::DW_OP_LLVM_tag_offset:
     case dwarf::DW_OP_LLVM_fragment:
+    case dwarf::DW_OP_LLVM_arg:
       continue;
     default:
       return true;
@@ -1441,6 +1445,36 @@ bool DIExpression::isComplex() const {
   return false;
 }
 
+bool DIExpression::isSingleLocationExpression() const {
+  if (!isValid())
+    return false;
+
+  if (getNumElements() == 0)
+    return true;
+
+  auto ExprOpBegin = expr_ops().begin();
+  auto ExprOpEnd = expr_ops().end();
+  if (ExprOpBegin->getOp() == dwarf::DW_OP_LLVM_arg)
+    ++ExprOpBegin;
+
+  return !std::any_of(ExprOpBegin, ExprOpEnd, [](auto Op) {
+    return Op.getOp() == dwarf::DW_OP_LLVM_arg;
+  });
+}
+
+const DIExpression *
+DIExpression::convertToVariadicExpression(const DIExpression *Expr) {
+  if (any_of(Expr->expr_ops(), [](auto ExprOp) {
+        return ExprOp.getOp() == dwarf::DW_OP_LLVM_arg;
+      }))
+    return Expr;
+  SmallVector<uint64_t> NewOps;
+  NewOps.reserve(Expr->getNumElements() + 2);
+  NewOps.append({dwarf::DW_OP_LLVM_arg, 0});
+  NewOps.append(Expr->elements_begin(), Expr->elements_end());
+  return DIExpression::get(Expr->getContext(), NewOps);
+}
+
 void DIExpression::canonicalizeExpressionOps(SmallVectorImpl<uint64_t> &Ops,
                                              const DIExpression *Expr,
                                              bool IsIndirect) {
@@ -1596,10 +1630,21 @@ DIExpression *DIExpression::appendOpsToArg(const DIExpression *Expr,
 
   SmallVector<uint64_t, 8> NewOps;
   for (auto Op : Expr->expr_ops()) {
+    // A DW_OP_stack_value comes at the end, but before a DW_OP_LLVM_fragment.
+    if (StackValue) {
+      if (Op.getOp() == dwarf::DW_OP_stack_value)
+        StackValue = false;
+      else if (Op.getOp() == dwarf::DW_OP_LLVM_fragment) {
+        NewOps.push_back(dwarf::DW_OP_stack_value);
+        StackValue = false;
+      }
+    }
     Op.appendToVector(NewOps);
     if (Op.getOp() == dwarf::DW_OP_LLVM_arg && Op.getArg(0) == ArgNo)
       NewOps.insert(NewOps.end(), Ops.begin(), Ops.end());
   }
+  if (StackValue)
+    NewOps.push_back(dwarf::DW_OP_stack_value);
 
   return DIExpression::get(Expr->getContext(), NewOps);
 }

llvm/lib/Transforms/Utils/Local.cpp

@@ -1908,8 +1908,8 @@ void llvm::salvageDebugInfoForDbgValues(
       DII->addVariableLocationOps(AdditionalValues, SalvagedExpr);
     } else {
       // Do not salvage using DIArgList for dbg.addr/dbg.declare, as it is
-      // currently only valid for stack value expressions. Do not salvage
-      // using DIArgList for dbg.assign yet. FIXME: support this.
+      // not currently supported in those instructions. Do not salvage using
+      // DIArgList for dbg.assign yet. FIXME: support this.
       // Also do not salvage if the resulting DIArgList would contain an
       // unreasonably large number of values.
       DII->setKillLocation();