[AMDGPU] Enable merging m0 initializations.

[kerbowa]

Summary

Enable hoisting and merging m0 defs that are initialized with the
same immediate value. Fixes bug where removed instructions are
not consider to interfere with other inits, and make sure to not
host inits before block prologs.

[rampitec]

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

509–510

Source alignment is off.

llvm/test/CodeGen/AMDGPU/merge-m0.mir

143

Most of these attributes can be removed.

[arsenm]

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

458–459

Why does this need to care about the prolog instructions? (FYI I'm planning on eliminating the rarely followed concept)

[rampitec]

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

458–459

You actually cannot, it was created to prevent RA from insrting spills into endif/else block before we updated exec mask.

[arsenm]

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

458–459

Yes, I'm working on splitting the blocks to avoid this

[rampitec]

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

458–459

I never found a way to properly split a block for that. Assume you split it and endif sequence is in the 1st block. What would prevent RA from inserting spill/restore there? You will need to mark a whole block as not suitable for spills and in addition prohibit RA from inserting any COPY instructions there.

Besides until there is another way m0 init must follow prologue anyway. If it is inserted before endif sequence that will break a contiguous sequence of prologue instructions and we will have bogus spilling again.

[kerbowa]

Fix indent, prolog->prologue, match increment style, remove attributes from test.

[arsenm]

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

458–459

The details of the split aren't important here. Why does this scalar value initialization care?

[kerbowa]

Update test.

[rampitec]

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

458–459

It does not care itself. But if inserted it will break prologue, allowing to insert a spill right before it.

[rampitec]

LGTM

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

 
 using namespace llvm;
 
 #define DEBUG_TYPE "si-fix-sgpr-copies"
 
 static cl::opt<bool> EnableM0Merge(
   "amdgpu-enable-merge-m0",
   cl::desc("Merge and hoist M0 initializations"),
-  cl::init(false));
+  cl::init(true));
 
 namespace {
 
 class SIFixSGPRCopies : public MachineFunctionPass {
   MachineDominatorTree *MDT;
 
 public:
   static char ID;
static bool isReachable(const MachineInstr *From,
 
   // Instructions are in different blocks, do predecessor search.
   // We should almost never get here since we do not usually produce M0 stores
   // other than -1.
   return searchPredecessors(MBBTo, CutOff, [MBBFrom]
            (const MachineBasicBlock *MBB) { return MBB == MBBFrom; });
 }
 
+// Return the first non-prologue instruction in the block.
+static MachineBasicBlock::iterator
+getFirstNonPrologue(MachineBasicBlock *MBB, const TargetInstrInfo *TII) {
+  MachineBasicBlock::iterator I = MBB->getFirstNonPHI();
+  while (I != MBB->end() && TII->isBasicBlockPrologue(*I))
+    ++I;
+
+  return I;
+}
+
 // Hoist and merge identical SGPR initializations into a common predecessor.
 // This is intended to combine M0 initializations, but can work with any
 // SGPR. A VGPR cannot be processed since we cannot guarantee vector
 // executioon.
 static bool hoistAndMergeSGPRInits(unsigned Reg,
                                    const MachineRegisterInfo &MRI,
-                                   MachineDominatorTree &MDT) {
+                                   MachineDominatorTree &MDT,
+                                   const TargetInstrInfo *TII) {
   // List of inits by immediate value.
   using InitListMap = std::map<unsigned, std::list<MachineInstr *>>;
   InitListMap Inits;
   // List of clobbering instructions.
   SmallVector<MachineInstr*, 8> Clobbers;
+  // List of instructions marked for deletion.
+  SmallSet<MachineInstr*, 8> MergedInstrs;
+
   bool Changed = false;
 
   for (auto &MI : MRI.def_instructions(Reg)) {
     MachineOperand *Imm = nullptr;
     for (auto &MO: MI.operands()) {
       if ((MO.isReg() && ((MO.isDef() && MO.getReg() != Reg) || !MO.isDef())) ||
           (!MO.isImm() && !MO.isReg()) || (MO.isImm() && Imm)) {
         Imm = nullptr;
for (auto &Init : Inits) {
 
     for (auto I1 = Defs.begin(), E = Defs.end(); I1 != E; ) {
       MachineInstr *MI1 = *I1;
 
       for (auto I2 = std::next(I1); I2 != E; ) {
         MachineInstr *MI2 = *I2;
 
         // Check any possible interference
-        auto intereferes = [&](MachineBasicBlock::iterator From,
-                               MachineBasicBlock::iterator To) -> bool {
+        auto interferes = [&](MachineBasicBlock::iterator From,
 
           assert(MDT.dominates(&*To, &*From));
 
           auto interferes = [&MDT, From, To](MachineInstr* &Clobber) -> bool {
             const MachineBasicBlock *MBBFrom = From->getParent();
             const MachineBasicBlock *MBBTo = To->getParent();
             bool MayClobberFrom = isReachable(Clobber, &*From, MBBTo, MDT);
             bool MayClobberTo = isReachable(Clobber, &*To, MBBTo, MDT);
for (auto I1 = Defs.begin(), E = Defs.end(); I1 != E; ) {
           return (llvm::any_of(Clobbers, interferes)) ||
                  (llvm::any_of(Inits, [&](InitListMap::value_type &C) {
                     return C.first != Init.first &&
                            llvm::any_of(C.second, interferes);
                   }));
         };
 
         if (MDT.dominates(MI1, MI2)) {
-          if (!intereferes(MI2, MI1)) {
+          if (!interferes(MI2, MI1)) {
             LLVM_DEBUG(dbgs()
                        << "Erasing from "
                        << printMBBReference(*MI2->getParent()) << " " << *MI2);
-            MI2->eraseFromParent();
-            Defs.erase(I2++);
+            MergedInstrs.insert(MI2);
             Changed = true;
+            ++I2;
             continue;
           }
         } else if (MDT.dominates(MI2, MI1)) {
-          if (!intereferes(MI1, MI2)) {
+          if (!interferes(MI1, MI2)) {
             LLVM_DEBUG(dbgs()
                        << "Erasing from "
                        << printMBBReference(*MI1->getParent()) << " " << *MI1);
-            MI1->eraseFromParent();
-            Defs.erase(I1++);
+            MergedInstrs.insert(MI1);
             Changed = true;
+            ++I1;
             break;
           }
         } else {
           auto *MBB = MDT.findNearestCommonDominator(MI1->getParent(),
                                                      MI2->getParent());
           if (!MBB) {
             ++I2;
             continue;
           }
 
-          MachineBasicBlock::iterator I = MBB->getFirstNonPHI();
-          if (!intereferes(MI1, I) && !intereferes(MI2, I)) {
+          MachineBasicBlock::iterator I = getFirstNonPrologue(MBB, TII);
+          if (!interferes(MI1, I) && !interferes(MI2, I)) {
             LLVM_DEBUG(dbgs()
                        << "Erasing from "
                        << printMBBReference(*MI1->getParent()) << " " << *MI1
                        << "and moving from "
                        << printMBBReference(*MI2->getParent()) << " to "
                        << printMBBReference(*I->getParent()) << " " << *MI2);
             I->getParent()->splice(I, MI2->getParent(), MI2);
-            MI1->eraseFromParent();
-            Defs.erase(I1++);
+            MergedInstrs.insert(MI1);
             Changed = true;
+            ++I1;
             break;
           }
         }
         ++I2;
       }
       ++I1;
     }
   }
 
+  for (auto MI : MergedInstrs)
+    MI->removeFromParent();
+
   if (Changed)
     MRI.clearKillFlags(Reg);
 
   return Changed;
 }
 
 bool SIFixSGPRCopies::runOnMachineFunction(MachineFunction &MF) {
   const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
for (MachineBasicBlock::iterator I = MBB.begin(), E = MBB.end();
         }
         break;
       }
       }
     }
   }
 
   if (MF.getTarget().getOptLevel() > CodeGenOpt::None && EnableM0Merge)
-    hoistAndMergeSGPRInits(AMDGPU::M0, MRI, *MDT);
+    hoistAndMergeSGPRInits(AMDGPU::M0, MRI, *MDT, TII);
 
   return true;
 }

llvm/test/CodeGen/AMDGPU/merge-m0.mir

 # GCN:      SI_INIT_M0 %2
 # GCN-NEXT: DS_WRITE_B32
 # GCN-NEXT: SI_INIT_M0 %2
 # GCN-NEXT: DS_WRITE_B32
 # GCN-NEXT: SI_INIT_M0 -1
 # GCN-NEXT: DS_WRITE_B32
 
 ---
-name:            test
-alignment:       0
-exposesReturnsTwice: false
-legalized:       false
-regBankSelected: false
-selected:        false
-tracksRegLiveness: true
+name:            merge-m0-many-init
 registers:
   - { id: 0, class: vgpr_32 }
   - { id: 1, class: vgpr_32 }
   - { id: 2, class: sreg_32_xm0 }
 body:             |
   bb.0.entry:
     successors: %bb.1, %bb.2
 
bb.6:
     SI_INIT_M0 %2, implicit-def $m0
     DS_WRITE_B32 %0, %1, 0, 0, implicit $m0, implicit $exec
     SI_INIT_M0 -1, implicit-def $m0
     DS_WRITE_B32 %0, %1, 0, 0, implicit $m0, implicit $exec
     S_CBRANCH_VCCZ %bb.6, implicit undef $vcc
     S_BRANCH %bb.0.entry
 
 ...
+
+# GCN:    bb.0.entry:
+# GCN:      SI_INIT_M0 65536
+# GCN-NEXT: DS_WRITE_B32
+
+#GCN:     bb.1:
+#GCN-NOT:   SI_INIT_M0 65536
+#GCN-NOT:   SI_INIT_M0 -1
+
+#GCN:     bb.2:
+#GCN:       SI_INIT_M0 -1
+
+#GCN:     bb.3:
+#GCN:       SI_INIT_M0 -1
+
+---
+name:            merge-m0-dont-hoist-past-init-with-different-initializer
+registers:
+  - { id: 0, class: vgpr_32 }
+  - { id: 1, class: vgpr_32 }
+body:             |
+  bb.0.entry:
+    successors: %bb.1
+
+    %0 = IMPLICIT_DEF
+    %1 = IMPLICIT_DEF
+    SI_INIT_M0 65536, implicit-def $m0
+    DS_WRITE_B32 %0, %1, 0, 0, implicit $m0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.2, %bb.3
+
+    SI_INIT_M0 65536, implicit-def $m0
+    DS_WRITE_B32 %0, %1, 0, 0, implicit $m0, implicit $exec
+    S_CBRANCH_VCCZ %bb.2, implicit undef $vcc
+    S_BRANCH %bb.3
+
+  bb.2:
+    successors: %bb.4
+
+    SI_INIT_M0 -1, implicit-def $m0
+    DS_WRITE_B32 %0, %1, 0, 0, implicit $m0, implicit $exec
+    S_BRANCH %bb.4
+
+  bb.3:
+    successors: %bb.4
+
+    SI_INIT_M0 -1, implicit-def $m0
+    DS_WRITE_B32 %0, %1, 0, 0, implicit $m0, implicit $exec
+    S_BRANCH %bb.4
+
+  bb.4:
+    S_ENDPGM 0
+...
+
+# GCN:    bb.0.entry:
+# GCN-NOT:  SI_INIT_M0
+# GCN:      S_OR_B64
+# GCN-NEXT: SI_INIT_M0
+
+#GCN:     bb.1:
+#GCN-NOT:   SI_INIT_M0 -1
+
+#GCN:     bb.2:
+#GCN-NOT:   SI_INIT_MO -1
+
+---
+name:            merge-m0-after-prologue
+registers:
+  - { id: 0, class: vgpr_32 }
+  - { id: 1, class: vgpr_32 }
+body:             |
+  bb.0.entry:
+    successors: %bb.1, %bb.2
+    liveins: $sgpr0_sgpr1
+
+    $exec = S_OR_B64 $exec, killed $sgpr0_sgpr1, implicit-def $scc
+    %0 = IMPLICIT_DEF
+    %1 = IMPLICIT_DEF
+    S_CBRANCH_VCCZ %bb.1, implicit undef $vcc
+    S_BRANCH %bb.2
+
+  bb.1:
+    successors: %bb.3
+
+    SI_INIT_M0 -1, implicit-def $m0
+    DS_WRITE_B32 %0, %1, 0, 0, implicit $m0, implicit $exec
+    S_BRANCH %bb.3
+
+  bb.2:
+    successors: %bb.3
+
+    SI_INIT_M0 -1, implicit-def $m0
+    DS_WRITE_B32 %0, %1, 0, 0, implicit $m0, implicit $exec
+    S_BRANCH %bb.3
+
+  bb.3:
+    S_ENDPGM 0
+...