[DebugInstrRef][1/9] Initial fields and instruction for instruction-referencing variable locations

[jmorse]

Summary

This is a series of patches adding basic instruction-referencing functionality to debug instructions, as described in this RFC [0]. All of this work hinges on the stack of LiveDebugValues work at D83046 which is getting reviewed: it may or may not finish anytime soon, but I've uploaded these patches for further transparency and reducing bus factor.

This patch series allows DBG_INSTR_REF instructions to reference instructions that are (initially) in the same block as the corresponding dbg.value; two other series will deal with:

• PHIs, and their passage through regalloc,
• COPYs.

This patch itself doesn't add any code, instead it adds some data structure fields: each MachineInstr now gets an instruction number field, and a corresponding field in MachineFunction records the "next" instruction number. The idea behind this is that each MachineInstr is going to carry around a unique identifier that DBG_INSTR_REFs refer to. This is the core part of the "lets refer to instructions" idea. Each DBG_INSTR_REF instruction has the format:

DBG_INSTR_REF inst-num-imm, operand-num-imm, !DILocalVariable..., !DIExpression...

i.e., the first operand identifies an instruction number and an operand within that instruction. This means we can identify values by the place that they're defined, but an important design decision is that there's no direct connection between the two. It's inefficient to identify the instruction from the DBG_INSTR_REF, and vice versa. This may (or may not) be worthy of discussion.

[0] http://lists.llvm.org/pipermail/llvm-dev/2020-February/139440.html

[aprantl]

Mechanically this looks fine — would be good for others to chime in whether we have consensus over doing this change though.

llvm/lib/CodeGen/MachineInstr.cpp

138

Use = 0 in the header instead?

[aprantl]

Oh, one important thing: Can you add a section to the .rst documentation with a MIR example that explains what a DBG_VALUE_REF is / what it's used for, etc?

[jmorse]

In D85741#2226310, @aprantl wrote:

Oh, one important thing: Can you add a section to the .rst documentation with a MIR example that explains what a DBG_VALUE_REF is / what it's used for, etc?

Curses, this fell out of my working-set before commiting; I'll cook that up now.

llvm/include/llvm/CodeGen/MachineFunction.h

private:
   void handleInsertion(MachineInstr &MI);
   void handleRemoval(MachineInstr &MI);
   friend struct ilist_traits<MachineInstr>;
 
 public:
   using VariableDbgInfoMapTy = SmallVector<VariableDbgInfo, 4>;
   VariableDbgInfoMapTy VariableDbgInfos;
 
+  /// A count of how many instructions in the function have had numbers
+  /// assigned to them. Used for debug value tracking, to determine the
+  /// next instruction number.
+  unsigned DebugInstrNumberingCount = 0;
+
   MachineFunction(Function &F, const LLVMTargetMachine &Target,
                   const TargetSubtargetInfo &STI, unsigned FunctionNum,
                   MachineModuleInfo &MMI);
   MachineFunction(const MachineFunction &) = delete;
   MachineFunction &operator=(const MachineFunction &) = delete;
   ~MachineFunction();
 
   /// Reset the instance as if it was just created.
const std::vector<char> &getBBSectionsSymbolPrefix() const {
     return BBSectionsSymbolPrefix;
   }
 
   /// Move the call site info from \p Old to \New call site info. This function
   /// is used when we are replacing one call instruction with another one to
   /// the same callee.
   void moveCallSiteInfo(const MachineInstr *Old,
                         const MachineInstr *New);
+
+  unsigned getNewDebugInstrNum() {
+    return ++DebugInstrNumberingCount;
+  }
 };
 
 //===--------------------------------------------------------------------===//
 // GraphTraits specializations for function basic block graphs (CFGs)
 //===--------------------------------------------------------------------===//
 
 // Provide specializations of GraphTraits to be able to treat a
 // machine function as a graph of machine basic blocks... these are

llvm/include/llvm/CodeGen/MachineInstr.h

PointerSumType<ExtraInfoInlineKinds,
                  PointerSumTypeMember<EIIK_MMO, MachineMemOperand *>,
                  PointerSumTypeMember<EIIK_PreInstrSymbol, MCSymbol *>,
                  PointerSumTypeMember<EIIK_PostInstrSymbol, MCSymbol *>,
                  PointerSumTypeMember<EIIK_OutOfLine, ExtraInfo *>>
       Info;
 
   DebugLoc debugLoc;                    // Source line information.
 
+  /// Unique instruction number. Used by DBG_INSTR_REFs to refer to the values
+  /// defined by this instruction.
+  unsigned DebugInstrNum;
+
   // Intrusive list support
   friend struct ilist_traits<MachineInstr>;
   friend struct ilist_callback_traits<MachineBasicBlock>;
   void setParent(MachineBasicBlock *P) { Parent = P; }
 
   /// This constructor creates a copy of the given
   /// MachineInstr in the given MachineFunction.
   MachineInstr(MachineFunction &, const MachineInstr &);
public:
   /// Return the complex address expression referenced by
   /// this DBG_VALUE instruction.
   const DIExpression *getDebugExpression() const;
 
   /// Return the debug label referenced by
   /// this DBG_LABEL instruction.
   const DILabel *getDebugLabel() const;
 
+  /// Fetch the instruction number of this MachineInstr. If it does not have
+  /// one already, a new and unique number will be assigned.
+  unsigned getDebugInstrNum();
+
+  /// Examine the instruction number of this MachineInstr. May be zero if
+  /// it hasn't been assigned a number yet.
+  unsigned peekDebugInstrNum() const { return DebugInstrNum; }
+
   /// Emit an error referring to the source location of this instruction.
   /// This should only be used for inline assembly that is somehow
   /// impossible to compile. Other errors should have been handled much
   /// earlier.
   ///
   /// If this method returns, the caller should try to recover from the error.
   void emitError(StringRef Msg) const;
 
bool isCFIInstruction() const {
     return getOpcode() == TargetOpcode::CFI_INSTRUCTION;
   }
 
   // True if the instruction represents a position in the function.
   bool isPosition() const { return isLabel() || isCFIInstruction(); }
 
   bool isDebugValue() const { return getOpcode() == TargetOpcode::DBG_VALUE; }
   bool isDebugLabel() const { return getOpcode() == TargetOpcode::DBG_LABEL; }
-  bool isDebugInstr() const { return isDebugValue() || isDebugLabel(); }
+  bool isDebugRef() const { return getOpcode() == TargetOpcode::DBG_INSTR_REF; }
+  bool isDebugInstr() const {
+    return isDebugValue() || isDebugLabel() || isDebugRef();
+  }
 
   bool isDebugOffsetImm() const { return getDebugOffset().isImm(); }
 
   /// A DBG_VALUE is indirect iff the location operand is a register and
   /// the offset operand is an immediate.
   bool isIndirectDebugValue() const {
     return isDebugValue() && getDebugOperand(0).isReg() && isDebugOffsetImm();
   }
bool isMetaInstruction() const {
     default:
       return false;
     case TargetOpcode::IMPLICIT_DEF:
     case TargetOpcode::KILL:
     case TargetOpcode::CFI_INSTRUCTION:
     case TargetOpcode::EH_LABEL:
     case TargetOpcode::GC_LABEL:
     case TargetOpcode::DBG_VALUE:
+    case TargetOpcode::DBG_INSTR_REF:
     case TargetOpcode::DBG_LABEL:
     case TargetOpcode::LIFETIME_START:
     case TargetOpcode::LIFETIME_END:
       return true;
     }
   }
 
   /// Return true if this is a transient instruction that is either very likely

llvm/include/llvm/Support/TargetOpcodes.def

 /// virtual registers have been created for all the instructions, and it's
 /// only needed in cases where the register classes implied by the
 /// instructions are insufficient. It is emitted as a COPY MachineInstr.
   HANDLE_TARGET_OPCODE(COPY_TO_REGCLASS)
 
 /// DBG_VALUE - a mapping of the llvm.dbg.value intrinsic
 HANDLE_TARGET_OPCODE(DBG_VALUE)
 
+/// DBG_INSTR_REF - A mapping of llvm.dbg.value referring to the instruction
+/// that defines the value, rather than a virtual register.
+HANDLE_TARGET_OPCODE(DBG_INSTR_REF)
+
 /// DBG_LABEL - a mapping of the llvm.dbg.label intrinsic
 HANDLE_TARGET_OPCODE(DBG_LABEL)
 
 /// REG_SEQUENCE - This variadic instruction is used to form a register that
 /// represents a consecutive sequence of sub-registers. It's used as a
 /// register coalescing / allocation aid and must be eliminated before code
 /// emission.
 // In SDNode form, the first operand encodes the register class created by

llvm/include/llvm/Target/Target.td

def COPY_TO_REGCLASS : StandardPseudoInstruction {
   let isAsCheapAsAMove = 1;
 }
 def DBG_VALUE : StandardPseudoInstruction {
   let OutOperandList = (outs);
   let InOperandList = (ins variable_ops);
   let AsmString = "DBG_VALUE";
   let hasSideEffects = 0;
 }
+def DBG_INSTR_REF : StandardPseudoInstruction {
+  let OutOperandList = (outs);
+  let InOperandList = (ins variable_ops);
+  let AsmString = "DBG_INSTR_REF";
+  let hasSideEffects = 0;
+}
 def DBG_LABEL : StandardPseudoInstruction {
   let OutOperandList = (outs);
   let InOperandList = (ins unknown:$label);
   let AsmString = "DBG_LABEL";
   let hasSideEffects = 0;
 }
 def REG_SEQUENCE : StandardPseudoInstruction {
   let OutOperandList = (outs unknown:$dst);

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

for (auto &MI : MBB) {
         emitInlineAsm(&MI);
         break;
       case TargetOpcode::DBG_VALUE:
         if (isVerbose()) {
           if (!emitDebugValueComment(&MI, *this))
             emitInstruction(&MI);
         }
         break;
+      case TargetOpcode::DBG_INSTR_REF:
+        // This instruction reference will have been resolved to a machine
+        // location, and a nearby DBG_VALUE created. We can safely ignore
+        // the instruction reference.
+        break;
       case TargetOpcode::DBG_LABEL:
         if (isVerbose()) {
           if (!emitDebugLabelComment(&MI, *this))
             emitInstruction(&MI);
         }
         break;
       case TargetOpcode::IMPLICIT_DEF:
         if (isVerbose()) emitImplicitDef(&MI);

llvm/lib/CodeGen/MachineInstr.cpp

for (const MCPhysReg *ImpUses = MCID->getImplicitUses(); *ImpUses;
       addOperand(MF, MachineOperand::CreateReg(*ImpUses, false, true));
 }
 
 /// MachineInstr ctor - This constructor creates a MachineInstr and adds the
 /// implicit operands. It reserves space for the number of operands specified by
 /// the MCInstrDesc.
 MachineInstr::MachineInstr(MachineFunction &MF, const MCInstrDesc &tid,
                            DebugLoc dl, bool NoImp)
-    : MCID(&tid), debugLoc(std::move(dl)) {
+    : MCID(&tid), debugLoc(std::move(dl)), DebugInstrNum(0) {
   assert(debugLoc.hasTrivialDestructor() && "Expected trivial destructor");
 
   // Reserve space for the expected number of operands.
   if (unsigned NumOps = MCID->getNumOperands() +
     MCID->getNumImplicitDefs() + MCID->getNumImplicitUses()) {
     CapOperands = OperandCapacity::get(NumOps);
     Operands = MF.allocateOperandArray(CapOperands);
   }
 
   if (!NoImp)
     addImplicitDefUseOperands(MF);
 }
 
-/// MachineInstr ctor - Copies MachineInstr arg exactly
-///
+/// MachineInstr ctor - Copies MachineInstr arg exactly.
+/// Does not copy the number from debug instruction numbering, to preserve
+/// uniqueness.
 MachineInstr::MachineInstr(MachineFunction &MF, const MachineInstr &MI)
-    : MCID(&MI.getDesc()), Info(MI.Info), debugLoc(MI.getDebugLoc()) {
+    : MCID(&MI.getDesc()), Info(MI.Info), debugLoc(MI.getDebugLoc()),
+      DebugInstrNum(0) {
   assert(debugLoc.hasTrivialDestructor() && "Expected trivial destructor");
 
   CapOperands = OperandCapacity::get(MI.getNumOperands());
   Operands = MF.allocateOperandArray(CapOperands);
 
   // Copy operands.
   for (const MachineOperand &MO : MI.operands())
     addOperand(MF, MO);

 }
 
 const DILabel *MachineInstr::getDebugLabel() const {
   assert(isDebugLabel() && "not a DBG_LABEL");
   return cast<DILabel>(getOperand(0).getMetadata());
 }
 
 const MachineOperand &MachineInstr::getDebugVariableOp() const {
-  assert(isDebugValue() && "not a DBG_VALUE");
+  assert((isDebugValue() || isDebugRef()) && "not a DBG_VALUE");
   return getOperand(2);
 }
 
 MachineOperand &MachineInstr::getDebugVariableOp() {
-  assert(isDebugValue() && "not a DBG_VALUE");
+  assert((isDebugValue() || isDebugRef()) && "not a DBG_VALUE");
   return getOperand(2);
 }
 
 const DILocalVariable *MachineInstr::getDebugVariable() const {
-  assert(isDebugValue() && "not a DBG_VALUE");
+  assert((isDebugValue() || isDebugRef()) && "not a DBG_VALUE");
   return cast<DILocalVariable>(getOperand(2).getMetadata());
 }
 
 MachineOperand &MachineInstr::getDebugExpressionOp() {
-  assert(isDebugValue() && "not a DBG_VALUE");
+  assert((isDebugValue() || isDebugRef()) && "not a DBG_VALUE");
   return getOperand(3);
 }
 
 const DIExpression *MachineInstr::getDebugExpression() const {
-  assert(isDebugValue() && "not a DBG_VALUE");
+  assert((isDebugValue() || isDebugRef()) && "not a DBG_VALUE");
   return cast<DIExpression>(getOperand(3).getMetadata());
 }
 
 bool MachineInstr::isDebugEntryValue() const {
   return isDebugValue() && getDebugExpression()->isEntryValue();
 }
 
 const TargetRegisterClass*
if (MDNode *HeapAllocMarker = getHeapAllocMarker()) {
     if (!FirstOp) {
       FirstOp = false;
       OS << ',';
     }
     OS << " heap-alloc-marker ";
     HeapAllocMarker->printAsOperand(OS, MST);
   }
 
+  if (DebugInstrNum) {
+    if (!FirstOp)
+      OS << ",";
+    OS << " debug-instr-number " << DebugInstrNum;
+  }
+
   if (!SkipDebugLoc) {
     if (const DebugLoc &DL = getDebugLoc()) {
       if (!FirstOp)
         OS << ',';
       OS << " debug-location ";
       DL->printAsOperand(OS, MST);
     }
   }

 
 Optional<unsigned>
 MachineInstr::getFoldedRestoreSize(const TargetInstrInfo *TII) const {
   MMOList Accesses;
   if (TII->hasLoadFromStackSlot(*this, Accesses))
     return getSpillSlotSize(Accesses, getMF()->getFrameInfo());
   return None;
 }
+
+unsigned MachineInstr::getDebugInstrNum() {
+  if (DebugInstrNum == 0)
+    DebugInstrNum = getParent()->getParent()->getNewDebugInstrNum();
+  return DebugInstrNum;
+}