Remove PrologEpilogInserter's usage of DBG_VALUE's offset field

[aprantl]

Summary

In the last half-dozen commits to LLVM I removed code that became dead after removing the offset parameter from llvm.dbg.value gradually proceeding from IR towards the backend. Before I can move on to DwarfDebug and friends there is one last side-called offset I need to remove:

This patch modifies PrologEpilogInserter's use of the DBG_VALUE's offset argument to use a DIExpression instead. Because the PrologEpilogInserter runs at the Machine level I had to play a little trick with a named llvm.dbg.mir node to get the DIExpressions to print in MIR dumps (which print the llvm::Module followed by the MachineFunction dump).

I also had to add rudimentary DwarfExpression support to CodeView and as a side-effect also fixed a bug (CodeViewDebug::collectVariableInfo was supposed to give up on variables with complex DIExpressions, but would fail to do so for fragments, which are also modeled as DIExpressions).

With this last holdover removed we will have only one canonical way of representing offsets to debug locations which will simplify the code in DwarfDebug (and future versions of CodeViewDebug once it starts handling more complex expressions) and make it easier to reason about.

This patch is NFC-ish: All test case changes are for assembler comments and the binary output does not change.

[rnk]

lgtm

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

992–997 ↗

(On Diff #109034)

It'd be nice if DIExpressionCursor had a helper for peeling everything that extractIfOffset reads.

llvm/trunk/include/llvm/CodeGen/MachineOperand.h

public:
   }
 
   void setIndex(int Idx) {
     assert((isFI() || isCPI() || isTargetIndex() || isJTI()) &&
            "Wrong MachineOperand mutator");
     Contents.OffsetedInfo.Val.Index = Idx;
   }
 
+  void setMetadata(const MDNode *MD) {
+    assert(isMetadata() && "Wrong MachineOperand mutator");
+    Contents.MD = MD;
+  }
+
   void setMBB(MachineBasicBlock *MBB) {
     assert(isMBB() && "Wrong MachineOperand mutator");
     Contents.MBB = MBB;
   }
 
   /// Sets value of register mask operand referencing Mask.  The
   /// operand does not take ownership of the memory referenced by Mask, it must
   /// remain valid for the lifetime of the operand. See CreateRegMask().

llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

 //===- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp ----------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains support for writing Microsoft CodeView debug info.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CodeViewDebug.h"
+#include "DwarfExpression.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallString.h"
for (const auto &I : DbgValues) {
     Var.DIVar = DIVar;
 
     // 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");
       const DIExpression *DIExpr = DVInst->getDebugExpression();
+      bool InMemory = DVInst->getOperand(1).isImm();
       bool IsSubfield = false;
       unsigned StructOffset = 0;
+      // Recognize a +Offset expression.
+      int Offset = 0;
+      DIExpressionCursor Ops(DIExpr);
+      auto Op = Ops.peek();
+      if (Op && Op->getOp() == dwarf::DW_OP_plus_uconst) {
+        Offset = Op->getArg(0);
+        Ops.take();
+      }
 
       // Handle fragments.
-      auto Fragment = DIExpr->getFragmentInfo();
+      auto Fragment = Ops.getFragmentInfo();
       if (Fragment) {
         IsSubfield = true;
         StructOffset = Fragment->OffsetInBits / 8;
-      } else if (DIExpr->getNumElements() > 0) {
-        continue; // Ignore unrecognized exprs.
       }
+      // Ignore unrecognized exprs.
+      if (Ops.peek() && Ops.peek()->getOp() != dwarf::DW_OP_LLVM_fragment)
+        continue;
+      if (!InMemory && Offset)
+        continue;
 
       // Bail if operand 0 is not a valid register. This means the variable is a
       // simple constant, or is described by a complex expression.
       // FIXME: Find a way to represent constant variables, since they are
       // relatively common.
       unsigned Reg =
           DVInst->getOperand(0).isReg() ? DVInst->getOperand(0).getReg() : 0;
       if (Reg == 0)
         continue;
 
       // Handle the two cases we can handle: indirect in memory and in register.
       unsigned CVReg = TRI->getCodeViewRegNum(Reg);
-      bool InMemory = DVInst->getOperand(1).isImm();
-      int Offset = InMemory ? DVInst->getOperand(1).getImm() : 0;
       {
         LocalVarDefRange DR;
         DR.CVRegister = CVReg;
         DR.InMemory = InMemory;
         DR.DataOffset = Offset;
         DR.IsSubfield = IsSubfield;
         DR.StructOffset = StructOffset;
 

llvm/trunk/lib/CodeGen/MachineModuleInfo.cpp

 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Pass.h"

 MachineModuleInfo::~MachineModuleInfo() = default;
 
 bool MachineModuleInfo::doInitialization(Module &M) {
   ObjFileMMI = nullptr;
   CurCallSite = 0;
   DbgInfoAvailable = UsesVAFloatArgument = UsesMorestackAddr = false;
   AddrLabelSymbols = nullptr;
   TheModule = &M;
-
+  if (getDebugMetadataVersionFromModule(M))
+    M.getOrInsertNamedMetadata("llvm.dbg.mir");
   return false;
 }
 
 bool MachineModuleInfo::doFinalization(Module &M) {
   Personalities.clear();
 
   delete AddrLabelSymbols;
   AddrLabelSymbols = nullptr;

llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp

 #include "llvm/CodeGen/MachineLoopInfo.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/CodeGen/RegisterScavenging.h"
 #include "llvm/CodeGen/StackProtector.h"
 #include "llvm/CodeGen/WinEHFuncInfo.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetFrameLowering.h"
 #include "llvm/Target/TargetInstrInfo.h"
for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
 
       // Frame indices in debug values are encoded in a target independent
       // way with simply the frame index and offset rather than any
       // target-specific addressing mode.
       if (MI.isDebugValue()) {
         assert(i == 0 && "Frame indices can only appear as the first "
                          "operand of a DBG_VALUE machine instruction");
         unsigned Reg;
-        MachineOperand &Offset = MI.getOperand(1);
-        Offset.setImm(
-            Offset.getImm() +
-            TFI->getFrameIndexReference(Fn, MI.getOperand(0).getIndex(), Reg));
+        int64_t Offset =
+            TFI->getFrameIndexReference(Fn, MI.getOperand(0).getIndex(), Reg);
         MI.getOperand(0).ChangeToRegister(Reg, false /*isDef*/);
+        auto *DIExpr = DIExpression::prepend(MI.getDebugExpression(),
+                                             DIExpression::NoDeref, Offset);
+        MI.getOperand(3).setMetadata(DIExpr);
+        const Module *M = Fn.getMMI().getModule();
+        // Add the expression to the metadata graph so isn't lost in MIR dumps.
+        M->getNamedMetadata("llvm.dbg.mir")->addOperand(DIExpr);
         continue;
       }
 
       // TODO: This code should be commoned with the code for
       // PATCHPOINT. There's no good reason for the difference in
       // implementation other than historical accident.  The only
       // remaining difference is the unconditional use of the stack
       // pointer as the base register.

llvm/trunk/lib/IR/Verifier.cpp

void Verifier::visitGlobalAlias(const GlobalAlias &GA) {
 
   visitGlobalValue(GA);
 }
 
 void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
   // There used to be various other llvm.dbg.* nodes, but we don't support
   // upgrading them and we want to reserve the namespace for future uses.
   if (NMD.getName().startswith("llvm.dbg."))
-    AssertDI(NMD.getName() == "llvm.dbg.cu",
+    AssertDI(NMD.getName() == "llvm.dbg.cu" || NMD.getName() == "llvm.dbg.mir",
              "unrecognized named metadata node in the llvm.dbg namespace",
              &NMD);
   for (const MDNode *MD : NMD.operands()) {
     if (NMD.getName() == "llvm.dbg.cu")
       AssertDI(MD && isa<DICompileUnit>(MD), "invalid compile unit", &NMD, MD);
 
     if (!MD)
       continue;

llvm/trunk/test/CodeGen/ARM/debug-info-arg.ll

 ; RUN: llc < %s | FileCheck %s
 ; Test to check argument y's debug info uses FI
 ; Radar 10048772
 target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:32:64-v128:32:128-a0:0:32-n32"
 target triple = "thumbv7-apple-ios"
 
 %struct.tag_s = type { i32, i32, i32 }
 
 define void @foo(%struct.tag_s* nocapture %this, %struct.tag_s* %c, i64 %x, i64 %y, %struct.tag_s* nocapture %ptr1, %struct.tag_s* nocapture %ptr2) nounwind ssp "no-frame-pointer-elim"="true" !dbg !1 {
   tail call void @llvm.dbg.value(metadata %struct.tag_s* %this, metadata !5, metadata !DIExpression()), !dbg !20
   tail call void @llvm.dbg.value(metadata %struct.tag_s* %c, metadata !13, metadata !DIExpression()), !dbg !21
   tail call void @llvm.dbg.value(metadata i64 %x, metadata !14, metadata !DIExpression()), !dbg !22
   tail call void @llvm.dbg.value(metadata i64 %y, metadata !17, metadata !DIExpression()), !dbg !23
-;CHECK: @DEBUG_VALUE: foo:y <- [%R7+8]
+;CHECK: @DEBUG_VALUE: foo:y <- [DW_OP_plus_uconst 8] [%R7+0]
   tail call void @llvm.dbg.value(metadata %struct.tag_s* %ptr1, metadata !18, metadata !DIExpression()), !dbg !24
   tail call void @llvm.dbg.value(metadata %struct.tag_s* %ptr2, metadata !19, metadata !DIExpression()), !dbg !25
   %1 = icmp eq %struct.tag_s* %c, null, !dbg !26
   br i1 %1, label %3, label %2, !dbg !26
 
 ; <label>:2                                       ; preds = %0
   tail call void @foobar(i64 %x, i64 %y) nounwind, !dbg !28
   br label %3, !dbg !28

llvm/trunk/test/DebugInfo/ARM/PR16736.ll

 ; RUN: llc -filetype=asm < %s | FileCheck %s
 ; RUN: llc -filetype=obj < %s \
 ; RUN:   | llvm-dwarfdump - | FileCheck %s --check-prefix=DWARF
 ;
-; CHECK: @DEBUG_VALUE: h:x <- [%R{{.*}}+{{.*}}]
+; CHECK: @DEBUG_VALUE: h:x <- [DW_OP_plus_uconst {{.*}}] [%R{{.*}}+0]
 ; DWARF: Location description: {{7[0-9] [0-9]+ $}}
 ;                              DW_OP_breg. +..
 ; generated from:
 ; clang -cc1 -triple  thumbv7 -S -O1 arm.cpp  -g
 ;
 ; int f();
 ; void g(float);
 ; void h(int, int, int, int, float x) {

llvm/trunk/test/DebugInfo/COFF/pieces.ll

 ; ASM:         retq
 
 ; ASM-LABEL: bitpiece_spill: # @bitpiece_spill
 ; ASM:         #DEBUG_VALUE: bitpiece_spill:o <- [DW_OP_LLVM_fragment 0 32] 0
 ; ASM:         xorl    %ecx, %ecx
 ; ASM:         callq   g
 ; ASM:         movl    %eax, [[offset_o_x:[0-9]+]](%rsp)          # 4-byte Spill
 ; ASM: [[spill_o_x_start:\.Ltmp[0-9]+]]:
-; ASM:         #DEBUG_VALUE: bitpiece_spill:o <- [DW_OP_LLVM_fragment 32 32] [%RSP+[[offset_o_x]]]
+; ASM:         #DEBUG_VALUE: bitpiece_spill:o <- [DW_OP_plus_uconst [[offset_o_x]], DW_OP_LLVM_fragment 32 32] [%RSP+0]
 ; ASM:         #APP
 ; ASM:         #NO_APP
 ; ASM:         movl    [[offset_o_x]](%rsp), %eax          # 4-byte Reload
 ; ASM: [[spill_o_x_end:\.Ltmp[0-9]+]]:
 ; ASM:         retq
 
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID

llvm/trunk/test/DebugInfo/X86/bbjoin.ll

 ; RUN: llc -mtriple=x86_64-apple-macosx10.9.0 %s -stop-after=livedebugvars \
 ; RUN:   -o - | FileCheck %s
 ; Generated from:
 ; void g(int *);
 ; int f() {
 ;   int x = 23;
 ;   g(&x);
 ;   if (x == 42)
 ;     ++x;
 ;   return x; // check that x is not a constant here.
 ; }
+; CHECK: ![[EXPR:.*]] = !DIExpression(DW_OP_plus_uconst, 4, DW_OP_deref)
 ; CHECK: ![[X:.*]] = !DILocalVariable(name: "x",
 ; CHECK: bb.0.entry:
 ; CHECK:   DBG_VALUE 23, 0, ![[X]],
-; CHECK:   DBG_VALUE %rsp, 4, ![[X]]
+; CHECK:   DBG_VALUE %rsp, 0, ![[X]], ![[EXPR]],
 ; CHECK: bb.1.if.then:
 ; CHECK:   DBG_VALUE 43, 0, ![[X]],
 ; CHECK: bb.2.if.end:
 ; CHECK-NOT:  DBG_VALUE 23, 0, ![[X]],
 ; CHECK:   RETQ %eax
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"

llvm/trunk/test/DebugInfo/X86/dbg-value-frame-index.ll

 while.body:
   br i1 0, label %while.end, label %while.cond
 
 while.end:
   ret i1 true
 }
 
 ; CHECK-LABEL: test
-; CHECK:       #DEBUG_VALUE: test:w <- [%RSP+8]
+; CHECK:       #DEBUG_VALUE: test:w <- [DW_OP_plus_uconst 8] [%RSP+0]
 ; DWARF:       Location description: 77 08
 ;                                    DW_OP_breg7 +8
 
 declare i1 @fn(i64*, i64*, i64*, i8*, i64, i64*, i32*, i8*)
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!2,!3}

llvm/trunk/test/DebugInfo/X86/debug-loc-asan.ll

 ; with "clang++ -S -emit-llvm -mllvm -asan-skip-promotable-allocas=0 -fsanitize=address -O0 -g test.cc"
 
 ; The address of the (potentially now malloc'ed) alloca ends up
 ; in RDI, after which it is spilled to the stack. We record the
 ; spill OFFSET on the stack for checking the debug info below.
 ; CHECK: #DEBUG_VALUE: bar:y <- [DW_OP_deref] [%RDI+0]
 ; CHECK: movq %rdi, [[OFFSET:[0-9]+]](%rsp)
 ; CHECK-NEXT: [[START_LABEL:.Ltmp[0-9]+]]
-; CHECK-NEXT: #DEBUG_VALUE: bar:y <- [DW_OP_deref, DW_OP_deref]
+; CHECK-NEXT: #DEBUG_VALUE: bar:y <- [DW_OP_plus_uconst [[OFFSET]], DW_OP_deref, DW_OP_deref]
 ; This location should be valid until the end of the function.
 
 ; CHECK:        movq    %rbp, %rsp
 ; CHECK-NEXT: [[END_LABEL:.Ltmp[0-9]+]]:
 
 ; CHECK: .Ldebug_loc{{[0-9]+}}:
 ; We expect two location ranges for the variable.
 

llvm/trunk/test/DebugInfo/X86/pieces-4.ll

 ;                "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15");
 ;   return o.x;
 ; }
 
 ; CHECK-LABEL: bitpiece_spill:                         # @bitpiece_spill
 ; CHECK:               callq   g
 ; CHECK:               movl    %eax, [[offs:[0-9]+]](%rsp)          # 4-byte Spill
 ; CHECK:               #DEBUG_VALUE: bitpiece_spill:o <- [DW_OP_LLVM_fragment 32 32] 0
-; CHECK:               #DEBUG_VALUE: bitpiece_spill:o <- [DW_OP_LLVM_fragment 0 32] [%RSP+[[offs]]]
+; CHECK:               #DEBUG_VALUE: bitpiece_spill:o <- [DW_OP_plus_uconst [[offs]], DW_OP_LLVM_fragment 0 32] [%RSP+0]
 ; CHECK:               #APP
 ; CHECK:               #NO_APP
 ; CHECK:               movl    [[offs]](%rsp), %eax          # 4-byte Reload
 ; CHECK:               retq
 ; DWARF: Location description: {{77 .. 93 04 10 00 9f 93 04}}
 ; DW_OP_breg7+offs DW_OP_piece 4 DW_OP_constu 0 DW_OP_stack_value DW_OP_piece 4
 
 ; ModuleID = 't.c'