[DebugInstrRef][7/9] NFC: Separate collection of machine/variable values

[jmorse]

Summary

This patch adjusts _when_ something happens in LiveDebugValues / InstrRefBasedLDV, to make it more amenable to dealing with DBG_INSTR_REF instructions. There's no functional change.

In the current base InstrRefBasedLDV implementation, we collect the machine value-number transfer function for blocks at the same time as the variable-value transfer function. After solving machine value numbers, the variable-value transfer function is updated so that DBG_VALUEs of live-in registers have the correct value. The same would need to be done for DBG_INSTR_REFs, to connect instruction-references with machine value numbers.

Rather than writing more code for that, this patch makes things slightly simpler:

• We collect the (machine-value-number) transfer function and solve for machine value numbers,
• Then step through the MachineInstrs again collecting the variable value transfer function.

The change is that linking up variable values with how they're defined (as being in a register, or as an instruction reference) is done as we step through the MachineInstrs, instead of on-the-fly. This is slower (in terms of compile time performance) but makes the next patch much easier to understand.

[aprantl]

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

3041

can this be for (MachineBasicBlock &MBB : OrderToBB) ?

3050

What is CurInst used for?

[jmorse]

Switch a loop to being ranged-based.

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

3041

Yup (ish), updated to use a range based loop.

3050

Because I'm identifying each value by the instruction defining it, each instruction needs a unique number which I derive from its block number and position within that block. See the ValueIDNum class in D83047, every value number is a tuple {block-number, instruction-number, register-or-spill-number}. In this loop we have to step through those numbers, with the "current" block / instruction number in CurBB / CurInst.

I guess this is easily replaceable by passing those numbers to "process" and its helpers rather than storing it in the object, and that would closer match what the VarLoc LiveDebugValues does. I'll add this to a list of things to polish, unless it's important for this patch.

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

private:
   /// too.
   void performCopy(Register Src, Register Dst);
 
   void accumulateFragmentMap(MachineInstr &MI);
 
   /// Step through the function, recording register definitions and movements
   /// in an MLocTracker. Convert the observations into a per-block transfer
   /// function in \p MLocTransfer, suitable for using with the machine value
-  /// location dataflow problem. Do the same with VLoc trackers in \p VLocs,
-  /// although the precise machine value numbers can't be known until after
-  /// the machine value number problem is solved.
-  void produceTransferFunctions(MachineFunction &MF,
-                                SmallVectorImpl<MLocTransferMap> &MLocTransfer,
-                                unsigned MaxNumBlocks,
-                                SmallVectorImpl<VLocTracker> &VLocs);
+  /// location dataflow problem.
+  void
+  produceMLocTransferFunction(MachineFunction &MF,
+                              unsigned MaxNumBlocks);
 
   /// Solve the machine value location dataflow problem. Takes as input the
   /// transfer functions in \p MLocTransfer. Writes the output live-in and
   /// live-out arrays to the (initialized to zero) multidimensional arrays in
   /// \p MInLocs and \p MOutLocs. The outer dimension is indexed by block
   /// number, the inner by LocIdx.
   void mlocDataflow(ValueIDNum **MInLocs, ValueIDNum **MOutLocs,
                     SmallVectorImpl<MLocTransferMap> &MLocTransfer);
if (transferDebugValue(MI))
     return;
   if (transferRegisterCopy(MI))
     return;
   if (transferSpillOrRestoreInst(MI))
     return;
   transferRegisterDef(MI);
 }
 
-void InstrRefBasedLDV::produceTransferFunctions(
+void InstrRefBasedLDV::produceMLocTransferFunction(
     MachineFunction &MF, SmallVectorImpl<MLocTransferMap> &MLocTransfer,
-    unsigned MaxNumBlocks, SmallVectorImpl<VLocTracker> &VLocs) {
+    unsigned MaxNumBlocks) {
   // Because we try to optimize around register mask operands by ignoring regs
   // that aren't currently tracked, we set up something ugly for later: RegMask
   // operands that are seen earlier than the first use of a register, still need
   // to clobber that register in the transfer function. But this information
   // isn't actively recorded. Instead, we track each RegMask used in each block,
   // and accumulated the clobbered but untracked registers in each block into
   // the following bitvector. Later, if new values are tracked, we can add
   // appropriate clobbers.
for (auto &MBB : MF) {
     CurBB = MBB.getNumber();
     CurInst = 1;
 
     // Set all machine locations to a PHI value. For transfer function
     // production only, this signifies the live-in value to the block.
     MTracker->reset();
     MTracker->setMPhis(CurBB);
 
-    VTracker = &VLocs[CurBB];
-    VTracker->MBB = &MBB;
-
     // Step through each instruction in this block.
     for (auto &MI : MBB) {
       process(MI);
       // Also accumulate fragment map.
       if (MI.isDebugValue())
         accumulateFragmentMap(MI);
       ++CurInst;
     }
while (!Worklist.empty()) {
       // Join values from predecessors. Updates LiveInIdx, and writes output
       // into JoinedInLocs.
       bool InLocsChanged, DowngradeOccurred;
       std::tie(InLocsChanged, DowngradeOccurred) = vlocJoin(
           *MBB, LiveOutIdx, LiveInIdx, (FirstTrip) ? &VLOCVisited : nullptr,
           CurBB, VarsWeCareAbout, MOutLocs, MInLocs, InScopeBlocks,
           BlocksToExplore, JoinedInLocs);
 
-      auto &VTracker = AllTheVLocs[MBB->getNumber()];
       bool FirstVisit = VLOCVisited.insert(MBB).second;
 
       // Always explore transfer function if inlocs changed, or if we've not
       // visited this block before.
       InLocsChanged |= FirstVisit;
 
       // If a downgrade occurred, book us in for re-examination on the next
       // iteration.
       if (DowngradeOccurred && OnPending.insert(MBB).second)
         Pending.push(BBToOrder[MBB]);
 
-      // Patch up the variable value transfer function to use the live-in
-      // machine values, now that that problem is solved.
-      if (FirstVisit) {
-        for (auto &Transfer : VTracker.Vars) {
-          if (Transfer.second.Kind == DbgValue::Def &&
-              Transfer.second.ID.getBlock() == CurBB &&
-              Transfer.second.ID.isPHI()) {
-            LocIdx Loc = Transfer.second.ID.getLoc();
-            Transfer.second.ID = MInLocs[CurBB][Loc.asU64()];
-          }
-        }
-      }
-
       if (!InLocsChanged)
         continue;
 
       // Do transfer function.
+      auto &VTracker = AllTheVLocs[MBB->getNumber()];
       for (auto &Transfer : VTracker.Vars) {
         // Is this var we're mangling in this scope?
         if (VarsWeCareAbout.count(Transfer.first)) {
           // Erase on empty transfer (DBG_VALUE $noreg).
           if (Transfer.second.Kind == DbgValue::Undef) {
             JoinedInLocs.erase(Transfer.first);
           } else {
             // Insert new variable value; or overwrite.
bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF,
   ++MaxNumBlocks;
 
   MLocTransfer.resize(MaxNumBlocks);
   vlocs.resize(MaxNumBlocks);
   SavedLiveIns.resize(MaxNumBlocks);
 
   initialSetup(MF);
 
-  produceTransferFunctions(MF, MLocTransfer, MaxNumBlocks, vlocs);
+  produceMLocTransferFunction(MF, MLocTransfer, MaxNumBlocks);
 
   // Allocate and initialize two array-of-arrays for the live-in and live-out
   // machine values. The outer dimension is the block number; while the inner
   // dimension is a LocIdx from MLocTracker.
   ValueIDNum **MOutLocs = new ValueIDNum *[MaxNumBlocks];
   ValueIDNum **MInLocs = new ValueIDNum *[MaxNumBlocks];
   unsigned NumLocs = MTracker->getNumLocs();
   for (int i = 0; i < MaxNumBlocks; ++i) {
     MOutLocs[i] = new ValueIDNum[NumLocs];
     MInLocs[i] = new ValueIDNum[NumLocs];
   }
 
   // Solve the machine value dataflow problem using the MLocTransfer function,
   // storing the computed live-ins / live-outs into the array-of-arrays. We use
   // both live-ins and live-outs for decision making in the variable value
   // dataflow problem.
   mlocDataflow(MInLocs, MOutLocs, MLocTransfer);
 
+  // Walk back through each block / instruction, collecting DBG_VALUE
+  // instructions and recording what machine value their operands refer to.
+  for (auto &OrderPair : OrderToBB) {
+    MachineBasicBlock &MBB = *OrderPair.second;
+    CurBB = MBB.getNumber();
+    VTracker = &vlocs[CurBB];
+    VTracker->MBB = &MBB;
+    MTracker->loadFromArray(MInLocs[CurBB], CurBB);
+    CurInst = 1;
+    for (auto &MI : MBB) {
+      process(MI);
+      ++CurInst;
+    }
+    MTracker->reset();
+  }
+
   // Number all variables in the order that they appear, to be used as a stable
   // insertion order later.
   DenseMap<DebugVariable, unsigned> AllVarsNumbering;
 
   // Map from one LexicalScope to all the variables in that scope.
   DenseMap<const LexicalScope *, SmallSet<DebugVariable, 4>> ScopeToVars;
 
   // Map from One lexical scope to all blocks in that scope.