[WIP][DebugInfo] Use VReg users to find DBG_VALUEs for a value

[jmorse]

Summary

This is a work-in-progress related to PR38754 [0], uploaded for visibility rather than review.

This patch is ugly because the behaviour exposed by the patch is ugly. If you disable placeDbgValues (topic of [0]), the currently semi-legitimate assumption of collectDebugValues (that DBG_VALUEs always follow the values definition) is eliminated, meaning DBG_VALUEs of a vreg can be in any BB in the function.

On the whole this is what we want IMHO, because that might be the debug-model of the program. However, we get a crash in MachineSink if it tries to sink a DBG_VALUE in front of the value-definition (maybe an un-related use-before-def bug), and a variety of infinite loops if DBG_VALUEs are sunk out of other BBs.

These are all matters that can be considered and fixed, they've just never been problems because placeDbgValues has squashed any kind of complicated DBG_VALUE layout. I've uploaded this patch because it allows clang/llvm to compile itself, might be a starting point for making other improvements. I've also only tested it with D54716 reducing the amount of code that uses collectDebugValues.

[0] https://bugs.llvm.org/show_bug.cgi?id=38754

[jmorse]

This was superseded by... well... trying to kill off collectDebugValues, basically. It's a general function that gets used in very specialised circumstances.

lib/CodeGen/MachineInstr.cpp

 }
 
 void MachineInstr::collectDebugValues(
                                 SmallVectorImpl<MachineInstr *> &DbgValues) {
   MachineInstr &MI = *this;
   if (!MI.getOperand(0).isReg())
     return;
 
-  MachineBasicBlock::iterator DI = MI; ++DI;
-  for (MachineBasicBlock::iterator DE = MI.getParent()->end();
-       DI != DE; ++DI) {
+  auto *MRI = getRegInfo();
+#ifndef NDEBUG
+  // There are no guarantees on DBG_VALUE availability and ordering after
+  // regalloc, they must be sought manually.
+  auto *TRI = MRI->getTargetRegisterInfo();
+  assert(!TRI->isPhysicalRegister(MI.getOperand(0).getReg()));
+#endif
+
+  for (auto &MO : MRI->use_operands(MI.getOperand(0).getReg())) {
+    auto *DI = MO.getParent();
     if (!DI->isDebugValue())
-      return;
+      continue;
+    // XXX XXX XXX numerous things explode if we "update" or move DBG_VALUEs
+    // from outside the current BB. These were previously ignored anyway by
+    // the prior implementation of collectDebugValues, so technically this
+    // isn't a regression...
+    if (DI->getParent() != getParent())
+      continue;
     if (DI->getOperand(0).isReg() &&
-        DI->getOperand(0).getReg() == MI.getOperand(0).getReg())
-      DbgValues.push_back(&*DI);
+        DI->getOperand(0).getReg() == MI.getOperand(0).getReg()) {
+      DbgValues.push_back(DI);
+    }
   }
 }
 
 void MachineInstr::changeDebugValuesDefReg(unsigned Reg) {
   // Collect matching debug values.
   SmallVector<MachineInstr *, 2> DbgValues;
   collectDebugValues(DbgValues);
 
   // Propagate Reg to debug value instructions.
   for (auto *DBI : DbgValues)
     DBI->getOperand(0).setReg(Reg);
 }

lib/CodeGen/MachineSink.cpp

bool MachineSinking::ProcessBlock(MachineBasicBlock &MBB) {
   // Walk the basic block bottom-up.  Remember if we saw a store.
   MachineBasicBlock::iterator I = MBB.end();
   --I;
   bool ProcessedBegin, SawStore = false;
   do {
     MachineInstr &MI = *I;  // The instruction to sink.
 
     // Predecrement I (if it's not begin) so that it isn't invalidated by
-    // sinking.
+    // sinking. Continue to decrement it past any debug insns, which might
+    // be sunk out of this BB.
+    do {
       ProcessedBegin = I == MBB.begin();
       if (!ProcessedBegin)
         --I;
+    } while (!ProcessedBegin && I->isDebugInstr());
 
     if (MI.isDebugInstr())
       continue;
 
     bool Joined = PerformTrivialForwardCoalescing(MI, &MBB);
     if (Joined) {
       MadeChange = true;
       continue;