[PATCH] D95748: AMDGPU: Fix dbg_value handling when forming soft clause bundles

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: rampitec, scott.linder, RamNalamothu, foad, dfukalov.
Herald added subscribers: kerbowa, hiraditya, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl.
arsenm requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

DBG_VALUES placed between memory instructions would change
codegen. Skip over these and re-insert them after the bundle instead
of giving up on bundling.


https://reviews.llvm.org/D95748

Files:
  llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
  llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp


Index: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp
+++ llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp
@@ -106,7 +106,8 @@
 // There no sense to create store clauses, they do not define anything,
 // thus there is nothing to set early-clobber.
 static bool isValidClauseInst(const MachineInstr &MI, bool IsVMEMClause) {
-  if (MI.isDebugValue() || MI.isBundled())
+  assert(!MI.isDebugInstr() && "debug instructions should not reach here");
+  if (MI.isBundled())
     return false;
   if (!MI.mayLoad() || MI.mayStore())
     return false;
@@ -321,6 +322,8 @@
   unsigned FuncMaxClause = AMDGPU::getIntegerAttribute(
       MF.getFunction(), "amdgpu-max-memory-clause", MaxClause);
 
+  SmallVector<MachineInstr *, 8> DbgInstrs;
+
   for (MachineBasicBlock &MBB : MF) {
     GCNDownwardRPTracker RPT(*LIS);
     MachineBasicBlock::instr_iterator Next;
@@ -328,6 +331,9 @@
       MachineInstr &MI = *I;
       Next = std::next(I);
 
+      if (MI.isDebugInstr())
+        continue;
+
       bool IsVMEM = isVMEMClauseInst(MI);
 
       if (!isValidClauseInst(MI, IsVMEM))
@@ -349,6 +355,13 @@
 
       unsigned Length = 1;
       for ( ; Next != E && Length < FuncMaxClause; ++Next) {
+        // Debug instructions should not change the bundling. We need to move
+        // these after the bundle
+        if (Next->isDebugInstr()) {
+          //DbgInstrs.push_back(&*Next);
+          continue;
+        }
+
         if (!isValidClauseInst(*Next, IsVMEM))
           break;
 
@@ -373,8 +386,17 @@
 
       // Restore the state after processing the bundle.
       RPT.reset(*B, &LiveRegsCopy);
+      DbgInstrs.clear();
+
+      auto BundleNext = I;
+      for (auto BI = I; BI != Next; BI = BundleNext) {
+        BundleNext = std::next(BI);
+
+        if (BI->isDebugValue()) {
+          DbgInstrs.push_back(BI->removeFromParent());
+          continue;
+        }
 
-      for (auto BI = I; BI != Next; ++BI) {
         BI->bundleWithPred();
         Ind->removeSingleMachineInstrFromMaps(*BI);
 
@@ -383,6 +405,10 @@
             MO.setIsInternalRead(true);
       }
 
+      // Replace any debug instructions after the new bundle.
+      for (MachineInstr *DbgInst : DbgInstrs)
+        MBB.insert(BundleNext, DbgInst);
+
       for (auto &&R : Defs) {
         forAllLanes(R.first, R.second.second, [&R, &B](unsigned SubReg) {
           unsigned S = R.second.first | RegState::EarlyClobber;
Index: llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -384,6 +384,7 @@
 
 void GCNDownwardRPTracker::advanceToNext() {
   LastTrackedMI = &*NextMI++;
+  NextMI = skipDebugInstructionsForward(NextMI, MBBEnd);
 
   // Add new registers or mask bits.
   for (const auto &MO : LastTrackedMI->operands()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D95748.320324.patch
Type: text/x-patch
Size: 2983 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210130/5ffd6fe0/attachment.bin>