[codeview] Generalize DIExpression parsing to handle load chains

[rnk]

Summary

Hopefully this also clarifies exactly when and why we're rewriting
certiain S_LOCALs using reference types: We're using the reference type
to stand in for a zero-offset load.

[inglorion]

lgtm modulo inline comment. Thanks!

llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp

81

Is this accidentally left in printstrumentation?

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

static LocalVarDefRange createDefRangeGeneral(uint16_t CVRegister,
                                                 bool InMemory, int Offset,
                                                 bool IsSubfield,
                                                 uint16_t StructOffset);
 
   /// Similar to DbgVariable in DwarfDebug, but not dwarf-specific.
   struct LocalVariable {
     const DILocalVariable *DIVar = nullptr;
     SmallVector<LocalVarDefRange, 1> DefRanges;
-    bool Deref = false;
+    bool UseReferenceType = false;
   };
 
   struct InlineSite {
     SmallVector<LocalVariable, 1> InlinedLocals;
     SmallVector<const DILocation *, 1> ChildSites;
     const DISubprogram *Inlinee = nullptr;
 
     /// The ID of the inline site or function used with .cv_loc. Not a type

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

for (const MachineFunction::VariableDbgInfo &VI : MF.getVariableDbgInfo()) {
 
     LocalVariable Var;
     Var.DIVar = VI.Var;
     Var.DefRanges.emplace_back(std::move(DefRange));
     recordLocalVariable(std::move(Var), VI.Loc->getInlinedAt());
   }
 }
 
+static bool canUseReferenceType(const DbgVariableLocation &Loc) {
+  return !Loc.LoadChain.empty() && Loc.LoadChain.back() == 0;
+}
+
+static bool needsReferenceType(const DbgVariableLocation &Loc) {
+  return Loc.LoadChain.size() == 2 && Loc.LoadChain.back() == 0;
+}
+
 void CodeViewDebug::calculateRanges(
     LocalVariable &Var, const DbgValueHistoryMap::InstrRanges &Ranges) {
   const TargetRegisterInfo *TRI = Asm->MF->getSubtarget().getRegisterInfo();
 
-  // calculate the definition ranges.
+  // Calculate the definition ranges.
   for (auto I = Ranges.begin(), E = Ranges.end(); I != E; ++I) {
     const InsnRange &Range = *I;
     const MachineInstr *DVInst = Range.first;
     assert(DVInst->isDebugValue() && "Invalid History entry");
     // FIXME: Find a way to represent constant variables, since they are
     // relatively common.
     Optional<DbgVariableLocation> Location =
         DbgVariableLocation::extractFromMachineInstruction(*DVInst);
     if (!Location)
       continue;
 
-    // Because we cannot express DW_OP_deref in CodeView directly,
-    // we use a trick: we encode the type as a reference to the
-    // real type.
-    if (Var.Deref) {
-      // When we're encoding the type as a reference to the original type,
-      // we need to remove a level of indirection from incoming locations.
-      // E.g. [RSP+8] with DW_OP_deref becomes [RSP+8],
-      // and [RCX+0] without DW_OP_deref becomes RCX.
-      if (!Location->Deref) {
-        if (Location->InMemory)
-          Location->InMemory = false;
-        else
-          continue;
-      }
-    } else if (Location->Deref) {
-      // We've encountered a Deref range when we had not applied the
-      // reference encoding. Start over using reference encoding.
-      Var.Deref = true;
+    // CodeView can only express variables in register and variables in memory
+    // at a constant offset from a register. However, for variables passed
+    // indirectly by pointer, it is common for that pointer to be spilled to a
+    // stack location. For the special case of one offseted load followed by a
+    // zero offset load (a pointer spilled to the stack), we change the type of
+    // the local variable from a value type to a reference type. This tricks the
+    // debugger into doing the load for us.
+    if (Var.UseReferenceType) {
+      // We're using a reference type. Drop the last zero offset load.
+      if (canUseReferenceType(*Location))
+        Location->LoadChain.pop_back();
+    } else if (needsReferenceType(*Location)) {
+      // This location can't be expressed without switching to a reference type.
+      // Start over using that.
+      Var.UseReferenceType = true;
       Var.DefRanges.clear();
       calculateRanges(Var, Ranges);
       return;
     }
 
-    // If we don't know how to handle this range, skip past it.
-    if (Location->Register == 0 || (Location->Offset && !Location->InMemory))
+    // We can only handle a register or an offseted load of a register.
+    if (Location->Register == 0 || Location->LoadChain.size() > 1)
       continue;
-
-    // Handle the two cases we can handle: indirect in memory and in register.
     {
       LocalVarDefRange DR;
       DR.CVRegister = TRI->getCodeViewRegNum(Location->Register);
-      DR.InMemory = Location->InMemory;
-      DR.DataOffset = Location->Offset;
+      DR.InMemory = !Location->LoadChain.empty();
+      DR.DataOffset =
+          !Location->LoadChain.empty() ? Location->LoadChain.back() : 0;
       if (Location->FragmentInfo) {
         DR.IsSubfield = true;
         DR.StructOffset = Location->FragmentInfo->OffsetInBits / 8;
       } else {
         DR.IsSubfield = false;
         DR.StructOffset = 0;
       }
 
void CodeViewDebug::emitLocalVariable(const LocalVariable &Var) {
 
   LocalSymFlags Flags = LocalSymFlags::None;
   if (Var.DIVar->isParameter())
     Flags |= LocalSymFlags::IsParameter;
   if (Var.DefRanges.empty())
     Flags |= LocalSymFlags::IsOptimizedOut;
 
   OS.AddComment("TypeIndex");
-  TypeIndex TI = Var.Deref ? getTypeIndexForReferenceTo(Var.DIVar->getType())
-                           : getCompleteTypeIndex(Var.DIVar->getType());
+  TypeIndex TI = Var.UseReferenceType
+                     ? getTypeIndexForReferenceTo(Var.DIVar->getType())
   OS.EmitIntValue(TI.getIndex(), 4);
   OS.AddComment("Flags");
   OS.EmitIntValue(static_cast<uint16_t>(Flags), 2);
   // Truncate the name so we won't overflow the record length field.
   emitNullTerminatedSymbolName(OS, Var.DIVar->getName());
   OS.EmitLabel(LocalEnd);
 
   // Calculate the on disk prefix of the appropriate def range record. The

llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.h

 namespace llvm {
 
 class AsmPrinter;
 class MachineInstr;
 class MachineModuleInfo;
 
 /// Represents the location at which a variable is stored.
 struct DbgVariableLocation {
-  /// Offset relative to base register.
-  int64_t Offset;
-
   /// Base register.
   unsigned Register;
 
-  /// If false, Register is the location. If true,
-  /// Register+Offset point at the location.
-  unsigned InMemory : 1;
-
-  /// If false, the location holds the variable's value.
-  /// If true, the location holds the variable's address.
-  unsigned Deref : 1;
+  /// Chain of offsetted loads necessary to load the value if it lives in
+  /// memory. Every load except for the last is pointer-sized.
+  SmallVector<int64_t, 1> LoadChain;
 
   /// Present if the location is part of a larger variable.
   llvm::Optional<llvm::DIExpression::FragmentInfo> FragmentInfo;
 
   /// Extract a VariableLocation from a MachineInstr.
   /// This will only work if Instruction is a debug value instruction
   /// and the associated DIExpression is in one of the supported forms.
   /// If these requirements are not met, the returned Optional will not

llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp

 DbgVariableLocation::extractFromMachineInstruction(
     const MachineInstr &Instruction) {
   DbgVariableLocation Location;
   if (!Instruction.isDebugValue())
     return None;
   if (!Instruction.getOperand(0).isReg())
     return None;
   Location.Register = Instruction.getOperand(0).getReg();
-  Location.InMemory = Instruction.getOperand(1).isImm();
-  Location.Deref = false;
   Location.FragmentInfo.reset();
   // We only handle expressions generated by DIExpression::appendOffset,
   // which doesn't require a full stack machine.
   int64_t Offset = 0;
   const DIExpression *DIExpr = Instruction.getDebugExpression();
   auto Op = DIExpr->expr_op_begin();
   while (Op != DIExpr->expr_op_end()) {
     switch (Op->getOp()) {
while (Op != DIExpr->expr_op_end()) {
     } break;
     case dwarf::DW_OP_plus_uconst:
       Offset += Op->getArg(0);
       break;
     case dwarf::DW_OP_LLVM_fragment:
       Location.FragmentInfo = {Op->getArg(1), Op->getArg(0)};
       break;
     case dwarf::DW_OP_deref:
-      Location.Deref = true;
+      Location.LoadChain.push_back(Offset);
+      Offset = 0;
       break;
     default:
       return None;
     }
     ++Op;
   }
 
-  Location.Offset = Offset;
+  // Do one final implicit DW_OP_deref if this was an indirect DBG_VALUE
+  // instruction.
+  // FIXME: Replace these with DIExpression.
+  if (Instruction.isIndirectDebugValue()) {
+    errs() << "adding load at offset for indirect value " << Offset << '\n';
+    Location.LoadChain.push_back(Offset);
+  }
+
   return Location;
 }
 
 DebugHandlerBase::DebugHandlerBase(AsmPrinter *A) : Asm(A), MMI(Asm->MMI) {}
 
 // Each LexicalScope has first instruction and last instruction to mark
 // beginning and end of a scope respectively. Create an inverse map that list
 // scopes starts (and ends) with an instruction. One instruction may start (or

llvm/test/CodeGen/MIR/X86/diexpr-win32.mir

body:             |
   bb.0.entry:
     liveins: %esi
 
     frame-setup PUSH32r killed %esi, implicit-def %esp, implicit %esp
     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, !11, debug-location !25
+    DBG_VALUE %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)
     MOV32mr %esi, 1, _, 0, _, killed %ecx, debug-location !29 :: (store 4 into %ir.0)
     MOV32mr %esi, 1, _, 4, _, killed %edx, debug-location !29 :: (store 4 into %ir.0 + 4)
     %eax = MOV32rm killed %eax, 1, _, 8, _, debug-location !29 :: (dereferenceable load 4 from %ir.1 + 8)
     MOV32mr %esi, 1, _, 8, _, killed %eax, debug-location !29 :: (store 4 into %ir.0 + 8)
     %eax = COPY killed %esi, debug-location !30
fixedStack:
   - { id: 1, type: default, offset: 0, size: 4, alignment: 4, stack-id: 0,
       isImmutable: true, isAliased: false, callee-saved-register: '' }
 stack:
 constants:
 body:             |
   bb.0.entry:
     %eax = MOV32rm %esp, 1, _, 4, _ :: (load 4 from %fixed-stack.1)
     %eax = MOV32rm killed %eax, 1, _, 0, _, debug-location !34 :: (load 4 from %ir.0)
-    DBG_VALUE debug-use %eax, 0, !35, !28, debug-location !34
+    DBG_VALUE debug-use %eax, 0, !35, !DIExpression(DW_OP_constu, 4, DW_OP_minus), debug-location !34
     %eax = ADD32rm killed %eax, %esp, 1, _, 8, _, implicit-def dead %eflags, debug-location !36 :: (load 4 from %fixed-stack.0)
     RET 0, %eax, debug-location !36
 
 ...