[WIP][DebugInfo] Fix SelectionDAGs placement of phi-reading dbg.values

[jmorse]

Summary

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

The test modified in this patch contained some FIXMEs added by the author when he noticed that DBG_VALUEs were being placed out of order. This patch fixes two of the FIXME'd placements, leaves one that isn't fixed, and marks up another incorrect DBG_VALUE placement that was pre-existing.

The cause is an interaction between source-IR-ordering and phi nodes: in the relevant BB, three phi nodes are read, which SelectionDAG pre-emptively allocates VRegs for. These are then brought into the DAG through CopyFromReg nodes. However, when emitted to a machine BB CopyFromReg doesn't create any instruction, and a nullptr entry is put into the IR-Location -> MachineInstr map. EmitSchedule can't place the DBG_VALUE there, so puts it one insn later than it would have otherwise.

The (rough) patch causes ProcessSourceNode to ignore SDNodes that didn't produce an instruction. That means subsequent SDNodes producing MachineInstrs get entered as valid locations in the IR-Location -> MachineInstr map, and EmitSchedule can emit DBG_VALUEs at the intended location.

The edge case is where an IR-Location / order-number doesn't have any MachineInstrs emitted at all: then the logic in EmitSchedule places DBG_VALUEs as early as it can, which is no different to how it behaves at the moment.

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

[jmorse]

This turned out to have PR40427 burried at the bottom of it, and was fixed in r352350.

lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp

 // to a vector which EmitSchedule uses to determine how to insert dbg_value
 // instructions in the right order.
 static void
 ProcessSourceNode(SDNode *N, SelectionDAG *DAG, InstrEmitter &Emitter,
                   DenseMap<SDValue, unsigned> &VRBaseMap,
                   SmallVectorImpl<std::pair<unsigned, MachineInstr*> > &Orders,
                   SmallSet<unsigned, 8> &Seen) {
   unsigned Order = N->getIROrder();
-  if (!Order || !Seen.insert(Order).second) {
+  if (!Order || Seen.count(Order) != 0) {
     // Process any valid SDDbgValues even if node does not have any order
     // assigned.
     ProcessSDDbgValues(N, DAG, Emitter, Orders, VRBaseMap, 0);
     return;
   }
 
   MachineBasicBlock *BB = Emitter.getBlock();
   auto IP = Emitter.getInsertPos();
   if (IP == BB->begin() || BB->back().isPHI() ||
       // Fast-isel may have inserted some instructions, in which case the
       // BB->back().isPHI() test will not fire when we want it to.
       std::prev(IP)->isPHI()) {
     // Did not insert any instruction.
-    Orders.push_back({Order, (MachineInstr *)nullptr});
+    //Orders.push_back({Order, (MachineInstr *)nullptr});
     return;
   }
 
+  Seen.insert(Order);
   Orders.push_back({Order, &*std::prev(IP)});
   ProcessSDDbgValues(N, DAG, Emitter, Orders, VRBaseMap, Order);
 }
 
 void ScheduleDAGSDNodes::
 EmitPhysRegCopy(SUnit *SU, DenseMap<SUnit*, unsigned> &VRBaseMap,
                 MachineBasicBlock::iterator InsertPos) {
   for (SUnit::const_pred_iterator I = SU->Preds.begin(), E = SU->Preds.end();

test/DebugInfo/X86/sdag-dbgvalue-phi-use-2.ll

 
 for.body:                                         ; preds = %for.body.lr.ph, %for.body
 ; CHECK-LABEL: bb.{{.*}}.for.body:
 ; CHECK:      [[REG2:%[0-9]+]]:gr32 = PHI
 ; CHECK-NEXT: [[REG3:%[0-9]+]]:gr32 = PHI
 ; CHECK-NEXT: [[REG4:%[0-9]+]]:gr32 = PHI
 ; CHECK-NEXT: DBG_VALUE [[REG3]], $noreg, !16
 ; CHECK-NEXT: DBG_VALUE 555, $noreg, !17
-; XXX: Shouldn't the following DBG_VALUE be placed after the add (ADD32rr).
+; CHECK-NEXT: [[ADDREG:%[0-9]+]]:gr32 = nuw nsw ADD32rr
 ; CHECK-NEXT: DBG_VALUE [[REG2]], $noreg, !17
-; CHECK-NEXT: ADD32rr
-; XXX: Shouldn't the following DBG_VALUE be placed after the mul (LEA etc).
+; CHECK:      [[MULREG:%[0-9]+]]:gr32 = LEA64_32r
 ; CHECK-NEXT: DBG_VALUE 777, $noreg, !17
-; CHECK:      INC32r
+; XXX: The following DBG_VALUE should have stayed below the INC32r
+; CHECK-NEXT: DBG_VALUE [[MULREG]], $noreg, !16
+; CHECK-NEXT: [[INCREG:%[0-9]+]]:gr32 = nuw nsw INC32r
+; CHECK-NEXT: DBG_VALUE [[ADDREG]], $noreg, !15
+; CHECK-NEXT: DBG_VALUE [[INCREG]], $noreg, !17
 ; XXX: Shouldn't the following DBG_VALUE be placed after the icmp (the non-dead implicit def of $eflags)
-; CHECK:      DBG_VALUE [[REG4]]
+; CHECK-NEXT: DBG_VALUE [[REG4]]
 ; CHECK-NEXT: implicit-def $eflags
   %u.023 = phi i32 [ 0, %for.body.lr.ph ], [ %inc, %for.body ]
   %y.022 = phi i32 [ 13, %for.body.lr.ph ], [ %mul, %for.body ]
   %x.021 = phi i32 [ 9, %for.body.lr.ph ], [ %add, %for.body ]
   call void @llvm.dbg.value(metadata i32 %y.022, metadata !16, metadata !DIExpression()), !dbg !27
   ; This dbg.value has been manually added compared to dbg_value_phi_isel1.ll
   call void @llvm.dbg.value(metadata i32 555, metadata !17, metadata !DIExpression()), !dbg !28
   %add = add nuw nsw i32 %y.022, %x.021, !dbg !41