[PATCH] D41864: DBG_VALUE insertion for spills breaks bundles

[Saurabh Verma via Phabricator via llvm-commits]

saurabhverma created this revision.
saurabhverma added reviewers: aprantl, llvm-commits, debug-info.
saurabhverma added a project: debug-info.

- follow up from llvm-dev email thread, summary below **
- Please note that this was discovered with a complex VLIW scheduling testcase in an out-of-tree target. I haven't managed to get an equivalent testcase for it with the upstream targets, but I am posting this patch to be taken up based on the fact that this error can (and does) result in inconsistent bundle flags and broken schedules. Even without the testcase, the code for DBG_VALUE insertion can be checked to see that this error can occur in the scenario described below  **

The insertion of DBG_VALUE instructions for spills does not seem to be handling insert locations inside bundles well. If the spill instruction is part of a bundle, the new DBG_VALUE is inserted after it, but does not have the bundling flags set. This essentially means that if we start with a set of bundled instructions:

MI1 [BundledSucc=true,  BundledPred=false]
MI2 [BundledSucc=false, BundledPred=true]

Where MI1 is a spill, and MI2 is a different instruction, after ExtendRanges we end up with

MI1 [BundledSucc=true,  BundledPred=false]
DBG_VALUE MI [BundledSucc=false,  BundledPred=false]
MI2 [BundledSucc=false, BundledPred=true]

Since this happens after the final instruction scheduling, it results in broken schedules. 
The patch checks if the DBG_VALUE node is inserted in the middle of a bundle and makes sure that its Bundle flags are set correctly, if so.


Repository:
  rL LLVM

https://reviews.llvm.org/D41864

Files:
  lib/CodeGen/LiveDebugValues.cpp


Index: lib/CodeGen/LiveDebugValues.cpp
===================================================================
--- lib/CodeGen/LiveDebugValues.cpp
+++ lib/CodeGen/LiveDebugValues.cpp
@@ -672,9 +672,16 @@
                                 /*transferSpills=*/true);
 
         // Add any DBG_VALUE instructions necessitated by spills.
-        for (auto &SP : Spills)
-          MBB->insertAfter(MachineBasicBlock::iterator(*SP.SpillInst),
-                           SP.DebugInst);
+        for (auto &SP : Spills){
+          MachineInstr *InsertPoint = SP.SpillInst;
+          MachineInstr *DebugInst = SP.DebugInst;
+          MBB->insertAfter(MachineBasicBlock::iterator(*InsertPoint),
+                           DebugInst);
+          if (InsertPoint->isBundledWithSucc()){
+            DebugInst->setFlag(MachineInstr::BundledSucc);
+            DebugInst->setFlag(MachineInstr::BundledPred);
+          }
+        }
         Spills.clear();
 
         DEBUG(printVarLocInMBB(MF, OutLocs, VarLocIDs,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41864.129080.patch
Type: text/x-patch
Size: 1002 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180109/4973a344/attachment.bin>