WIP: [LiveDebugValues] Support the debug entry values for modified parameters

[djtodoro]

Summary

Keep track of an entry value of a modified parameter if the modification of the parameter can be expressed in terms of its entry value and a constant (expression like (DW_OP_LLVM_entry_value reg1; DW_OP_plus_uconst, 7, DW_OP_stack_value) where we describe situation like param = param + 7;).

[djtodoro]

-Rebase

[aprantl]

llvm/include/llvm/IR/DebugInfoMetadata.h

2471

This doesn't seem to be a good API. I think this is supposed to recognize some specific high-level pattern, but doesn't really say so? Given how easily salvageDebugInfo keeps appending operations to DIExpressions, it seems very unlikely that this condition would be hit very often.

What's the use-case for this, and is there a better way to achieve the same result?

[aprantl]

llvm/include/llvm/IR/DebugInfoMetadata.h

2471

I.e., were are these expressions produced that we can be sure that we will recognize them with this function?

[djtodoro]

llvm/include/llvm/IR/DebugInfoMetadata.h

2471

Thanks for the comments! We are on the same page with this, that is why I left it as a 'WIP'. :)

From very high level, we want to recognize (simple) modifications of a parameter that could be expressed in terms of its entry value. By looking at the test provided, the problem is we have only the DBG_VALUE describing the modification (made from the Salvage Debug Info since the add instruction was deleted during the Combine redundant instructions pass). I am not sure if we should be aware what was the actual modification, we could just see if the DBG_VALUE register location is the same as the entry value and apply the DIExpression ops on to the entry value expression. I will examine that in details and be back with more info.

[djtodoro]

-Rebase

[djtodoro]

I will revisit this, since we need some more general approach.
(abandoning for now)

llvm/include/llvm/IR/DebugInfoMetadata.h

public:
 
   /// Return whether this is an implicit location description.
   bool isImplicit() const;
 
   /// Return whether the location is computed on the expression stack, meaning
   /// it cannot be a simple register location.
   bool isComplex() const;
 
+  ///  Return whether this is an operation with a constant offset.
+  bool isOpWithOffset() const;
+
   /// Append \p Ops with operations to apply the \p Offset.
   static void appendOffset(SmallVectorImpl<uint64_t> &Ops, int64_t Offset);
 
   /// If this is a constant offset, extract it. If there is no expression,
   /// return true with an offset of zero.
   bool extractIfOffset(int64_t &Offset) const;
 
   /// Checks if the last 4 elements of the expression are DW_OP_constu <DWARF

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

if (HasComplexExpression && DwarfRegs.size() > 1) {
     LocationKind = Unknown;
     return false;
   }
 
   // Handle simple register locations. If we are supposed to emit
   // a call site parameter expression and if that expression is just a register
   // location, emit it with addBReg and offset 0, because we should emit a DWARF
   // expression representing a value, rather than a location.
-  if (!isMemoryLocation() && !HasComplexExpression &&
-      (!isParameterValue() || isEntryValue())) {
+  if ((!isMemoryLocation() && !HasComplexExpression && !isParameterValue()) ||
+      isEntryValue()) {
     for (auto &Reg : DwarfRegs) {
       if (Reg.DwarfRegNo >= 0)
         addReg(Reg.DwarfRegNo, Reg.Comment);
       addOpPiece(Reg.Size);
     }
 
     if (isEntryValue())
       finalizeEntryValue();
 
-    if (isEntryValue() && !isParameterValue() && DwarfVersion >= 4)
-      emitOp(dwarf::DW_OP_stack_value);
+    if (isEntryValue() && !isParameterValue() && isRegisterLocation())
+      LocationKind = Implicit;
 
     DwarfRegs.clear();
     return true;
   }
 
   // Don't emit locations that cannot be expressed without DW_OP_stack_value.
   if (DwarfVersion < 4)
     if (any_of(ExprCursor, [](DIExpression::ExprOperand Op) -> bool {

llvm/lib/CodeGen/LiveDebugValues.cpp

#endif
   // record its debug entry value and potential movement of such value.
   struct EntryValue {
     // The debug entry value.
     MachineInstr *DbgEntryValue;
 
     // A copy like instruction that moves the entry value around.
     MachineInstr *DbgEntryValueMovement;
 
+    // A debug info expression describing the debug entry value modification.
+    DIExpression *DbgEntryValueModification;
+
     void setDbgEntryValueMovement(MachineInstr *MI) {
       DbgEntryValueMovement = MI;
     }
+
+    void setDbgEntryValueModification(DIExpression *Expr) {
+      DbgEntryValueModification = Expr;
+    }
   };
   using DebugParamMap = SmallDenseMap<const DILocalVariable *, EntryValue>;
 
   using VarLocMap = UniqueVector<VarLoc>;
   using VarLocSet = SparseBitVector<>;
   using VarLocInMBB = SmallDenseMap<const MachineBasicBlock *, VarLocSet>;
   struct TransferDebugPair {
     MachineInstr *TransferInst; /// Instruction where this transfer occurs.
void LiveDebugValues::removeOrSalvageEntryValue(const MachineInstr &MI,
   // entry value.
   if (DebugEntryVals[Var].DbgEntryValueMovement) {
     Register EntryValueCopyReg =
         DebugEntryVals[Var].DbgEntryValueMovement->getOperand(0).getReg();
     if (MI.getOperand(0).isReg() &&
         MI.getOperand(0).getReg() == EntryValueCopyReg) {
       if (MI.getDebugExpression()->getNumElements() == 0)
         return;
+      else if (Expr->isOpWithOffset()) {
+        // Handle the debug entry value modification.
+        DIExpression *NewEntryValExpr =
+            DebugEntryVals[Var].DbgEntryValueModification;
+        NewEntryValExpr =
+            DIExpression::prepend(Expr, DIExpression::ApplyOffset);
+        DebugEntryVals[Var].setDbgEntryValueModification(NewEntryValExpr);
+        return;
+      }
+      // Stop tracking the entry value.
+      DebugEntryVals.erase(Var);
     }
   }
 
   // Remove the debug entry value from the map if we encounter a different
   // DBG_VALUE describing the same parameter, because that can imply that the
   // parameter's value has changed, so we can not rely on its entry value any
   // more.
   Register EntryValueReg = DebugEntryValue->getOperand(0).getReg();
for (unsigned ID : KillSet) {
     if (ParamEntryValue == DebugEntryVals.end())
       continue;
 
     auto ParamDebugInstr = ParamEntryValue->second.DbgEntryValue;
     auto ParamEntryValMovement = ParamEntryValue->second.DbgEntryValueMovement;
     DIExpression *NewExpr = DIExpression::prepend(
         ParamDebugInstr->getDebugExpression(), DIExpression::EntryValue);
 
+    // Follow the modification of the debug entry value if any.
+    DIExpression *EntryValModification =
+        ParamEntryValue->second.DbgEntryValueModification;
+    if (EntryValModification->getNumElements() != 0) {
+      ArrayRef<uint64_t> elements = NewExpr->getElements();
+      SmallVector<uint64_t, 16> Ops;
+      Ops.append(elements.begin(), elements.end());
+      NewExpr = DIExpression::prependOpcodes(EntryValModification, Ops);
+    }
+
     auto getEntryLoc = [ParamDebugInstr, ParamEntryValMovement,
                         NewExpr](LexicalScopes &LS) {
       if (ParamEntryValMovement)
         return VarLoc::CreateCopyEntryLoc(
             *ParamDebugInstr, LS, NewExpr,
             ParamEntryValMovement->getOperand(0).getReg());
       return VarLoc::CreateEntryLoc(*ParamDebugInstr, LS, NewExpr);
     };
bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
 
   MachineBasicBlock &First_MBB = *(MF.begin());
   // Only in the case of entry MBB collect DBG_VALUEs representing
   // function parameters in order to generate debug entry values for them.
   // Currently, we generate debug entry values only for parameters that are
   // unmodified throughout the function and located in a register.
   // TODO: Add support for parameters with non-empty debug expressions (for
   // example fragments).
-  // TODO: Add support for modified arguments that can be expressed
-  // by using its entry value.
   // TODO: Add support for local variables that are expressed in terms of
   // parameters entry values.
   for (auto &MI : First_MBB)
     if (MI.isDebugValue() && !MI.getDebugLoc()->getInlinedAt() &&
         MI.isDebugValue() && MI.getDebugVariable()->isParameter() &&
         !MI.isIndirectDebugValue() &&
         isRegOtherThanSPAndFP(MI.getOperand(0), MI, TRI) &&
         !DebugEntryVals.count(MI.getDebugVariable()) &&
-        MI.getDebugExpression()->getNumElements() == 0)
-      DebugEntryVals[MI.getDebugVariable()] = EntryValue{&MI, nullptr};
+        MI.getDebugExpression()->getNumElements() == 0) {
+      DIExpression *EntryValueModificationExpr =
+          DIExpression::get(MF.getFunction().getContext(), {});
+      DebugEntryVals[MI.getDebugVariable()] =
+          EntryValue{&MI, nullptr, EntryValueModificationExpr};
+    }
 
   // Initialize per-block structures and scan for fragment overlaps.
   for (auto &MBB : MF) {
     PendingInLocs[&MBB] = VarLocSet();
 
     for (auto &MI : MBB) {
       if (MI.isDebugValue())
         accumulateFragmentMap(MI, SeenFragments, OverlapFragments);

llvm/lib/IR/DebugInfoMetadata.cpp

case dwarf::DW_OP_swap: {
       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 &&
-             getNumElements() == 2;
+      return I->get() == expr_op_begin()->get() && I->getArg(0) == 1;
     }
     case dwarf::DW_OP_LLVM_convert:
     case dwarf::DW_OP_LLVM_tag_offset:
     case dwarf::DW_OP_constu:
     case dwarf::DW_OP_plus_uconst:
     case dwarf::DW_OP_plus:
     case dwarf::DW_OP_minus:
     case dwarf::DW_OP_mul:
switch (It.getOp()) {
         continue;
       default: return true;
     }
   }
 
   return false;
 }
 
+bool DIExpression::isOpWithOffset() const {
+  if (getNumElements() == 0)
+    return false;
+
+  if (getNumElements() == 3 && Elements[0] == dwarf::DW_OP_plus_uconst)
+    return true;
+
+  if (getNumElements() == 4 && Elements[0] == dwarf::DW_OP_constu)
+    if (Elements[2] == dwarf::DW_OP_plus ||
+        Elements[2] == dwarf::DW_OP_minus)
+      return true;
+
+  return false;
+}
+
 Optional<DIExpression::FragmentInfo>
 DIExpression::getFragmentInfo(expr_op_iterator Start, expr_op_iterator End) {
   for (auto I = Start; I != End; ++I)
     if (I->getOp() == dwarf::DW_OP_LLVM_fragment) {
       DIExpression::FragmentInfo Info = {I->getArg(1), I->getArg(0)};
       return Info;
     }
   return None;

llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir

 #    return 1;
 #  else
 #    return 0;
 #}
 #
 # CHECK: DBG_VALUE $edi, $noreg, !16, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
 # CHECK: DBG_VALUE $edx, $noreg, !18, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
 # CHECK: DBG_VALUE $edi, $noreg, !16, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location {{.*}}
-# CHECK-NOT: DBG_VALUE $esi, $noreg, !17, !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 7, DW_OP_stack_value), debug-location {{.*}}
+# CHECK: DBG_VALUE $esi, $noreg, !17, !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 7, DW_OP_stack_value), debug-location {{.*}}
 #
 --- |
   ; ModuleID = 'test.c'
   source_filename = "test.c"
   target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
   target triple = "x86_64-unknown-linux-gnu"
   
   ; Function Attrs: noinline nounwind uwtable