[DebugInfo] LiveDebugValues should always revisit backedges if it skips them

The "join" method in LiveDebugValues does not attempt to join unseen
predecessor blocks if their out-locations aren't yet initialized, instead
the block should be re-visited later to see if any locations have changed
validity. However, because the set of blocks were all being "process"'d
once before "join" saw them, that logic in "join" was actually ignoring
legitimate out-locations on the first pass through. This meant that some
invalidated locations were not removed from the head of loops, allowing
illegal locations to persist.

Fix this by removing the run of "process" before the main join/process loop
in ExtendRanges. Now the unseen predecessors that "join" skips truly are
uninitialized, and we come back to the block at a later time to re-run
"join", see the @baz function added.

This also fixes another fault where stack/register transfers in the entry
block (or any other before-any-loop-block) had their tranfers initially
ignored, and were then never revisited. The MIR test added tests for this
behaviour.

XFail a test that exposes another bug; a fix for this is coming in D66895.

Differential Revision: https://reviews.llvm.org/D66663

llvm-svn: 370328

1 files changed, 17 insertions, 37 deletions

diff --git a/llvm/lib/CodeGen/LiveDebugValues.cpp b/llvm/lib/CodeGen/LiveDebugValues.cpp
index 1ddcceabe20..cd39950e392 100644
--- a/llvm/lib/CodeGen/LiveDebugValues.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues.cpp
@@ -385,7 +385,7 @@ private:
   void process(MachineInstr &MI, OpenRangesSet &OpenRanges,
                VarLocInMBB &OutLocs, VarLocMap &VarLocIDs,
                TransferMap &Transfers, DebugParamMap &DebugEntryVals,
-               bool transferChanges, OverlapMap &OverlapFragments,
+               OverlapMap &OverlapFragments,
                VarToFragments &SeenFragments);
 
   void accumulateFragmentMap(MachineInstr &MI, VarToFragments &SeenFragments,
@@ -1015,21 +1015,15 @@ void LiveDebugValues::accumulateFragmentMap(MachineInstr &MI,
 /// This routine creates OpenRanges and OutLocs.
 void LiveDebugValues::process(MachineInstr &MI, OpenRangesSet &OpenRanges,
                               VarLocInMBB &OutLocs, VarLocMap &VarLocIDs,
-                              TransferMap &Transfers, DebugParamMap &DebugEntryVals,
-                              bool transferChanges,
+                              TransferMap &Transfers,
+                              DebugParamMap &DebugEntryVals,
                               OverlapMap &OverlapFragments,
                               VarToFragments &SeenFragments) {
   transferDebugValue(MI, OpenRanges, VarLocIDs);
   transferRegisterDef(MI, OpenRanges, VarLocIDs, Transfers,
                       DebugEntryVals);
-  if (transferChanges) {
-    transferRegisterCopy(MI, OpenRanges, VarLocIDs, Transfers);
-    transferSpillOrRestoreInst(MI, OpenRanges, VarLocIDs, Transfers);
-  } else {
-    // Build up a map of overlapping fragments on the first run through.
-    if (MI.isDebugValue())
-      accumulateFragmentMap(MI, SeenFragments, OverlapFragments);
-  }
+  transferRegisterCopy(MI, OpenRanges, VarLocIDs, Transfers);
+  transferSpillOrRestoreInst(MI, OpenRanges, VarLocIDs, Transfers);
 }
 
 /// This routine joins the analysis results of all incoming edges in @MBB by
@@ -1050,9 +1044,11 @@ bool LiveDebugValues::join(
   // can be joined.
   int NumVisited = 0;
   for (auto p : MBB.predecessors()) {
-    // Ignore unvisited predecessor blocks.  As we are processing
-    // the blocks in reverse post-order any unvisited block can
-    // be considered to not remove any incoming values.
+    // Ignore backedges if we have not visited the predecessor yet. As the
+    // predecessor hasn't yet had locations propagated into it, most locations
+    // will not yet be valid, so treat them as all being uninitialized and
+    // potentially valid. If a location guessed to be correct here is
+    // invalidated later, we will remove it when we revisit this block.
     if (!Visited.count(p)) {
       LLVM_DEBUG(dbgs() << "  ignoring unvisited pred MBB: " << p->getNumber()
                         << "\n");
@@ -1215,8 +1211,6 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
                       std::greater<unsigned int>>
       Pending;
 
-  enum : bool { dontTransferChanges = false, transferChanges = true };
-
   // Besides parameter's modification, check whether a DBG_VALUE is inlined
   // in order to deduce whether the variable that it tracks comes from
   // a different function. If that is the case we can't track its entry value.
@@ -1254,27 +1248,14 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
         !MI.getDebugExpression()->isFragment())
       DebugEntryVals[MI.getDebugVariable()] = &MI;
 
-  // Initialize every mbb with OutLocs.
-  // We are not looking at any spill instructions during the initial pass
-  // over the BBs. The LiveDebugVariables pass has already created DBG_VALUE
-  // instructions for spills of registers that are known to be user variables
-  // within the BB in which the spill occurs.
+  // Initialize per-block structures and scan for fragment overlaps.
   for (auto &MBB : MF) {
+    PendingInLocs[&MBB] = VarLocSet();
+
     for (auto &MI : MBB) {
-      process(MI, OpenRanges, OutLocs, VarLocIDs, Transfers, DebugEntryVals,
-              dontTransferChanges, OverlapFragments, SeenFragments);
-    }
-    transferTerminator(&MBB, OpenRanges, OutLocs, VarLocIDs);
-    // Add any entry DBG_VALUE instructions necessitated by parameter
-    // clobbering.
-    for (auto &TR : Transfers) {
-      MBB.insertAfter(MachineBasicBlock::iterator(*TR.TransferInst),
-                     TR.DebugInst);
+      if (MI.isDebugValue())
+        accumulateFragmentMap(MI, SeenFragments, OverlapFragments);
     }
-    Transfers.clear();
-
-    // Initialize pending inlocs.
-    PendingInLocs[&MBB] = VarLocSet();
   }
 
   auto hasNonArtificialLocation = [](const MachineInstr &MI) -> bool {
@@ -1313,7 +1294,7 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
       Worklist.pop();
       MBBJoined = join(*MBB, OutLocs, InLocs, VarLocIDs, Visited,
                        ArtificialBlocks, PendingInLocs);
-      Visited.insert(MBB);
+      MBBJoined |= Visited.insert(MBB).second;
       if (MBBJoined) {
         MBBJoined = false;
         Changed = true;
@@ -1324,8 +1305,7 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
         OpenRanges.insertFromLocSet(PendingInLocs[MBB], VarLocIDs);
         for (auto &MI : *MBB)
               process(MI, OpenRanges, OutLocs, VarLocIDs, Transfers,
-                      DebugEntryVals, transferChanges, OverlapFragments,
-                      SeenFragments);
+                      DebugEntryVals, OverlapFragments, SeenFragments);
         OLChanged |= transferTerminator(MBB, OpenRanges, OutLocs, VarLocIDs);
 
         // Add any DBG_VALUE instructions necessitated by spills.