[AMDGPU] RA inserted scalar instructions can be at the BB top #72140

cdevadas · 2023-11-13T17:35:43Z

We adjust the insertion point at the BB top for spills/copies during RA to ensure they are placed after the exec restore instructions required for the divergent control flow execution. This is, however, required only for the vector operations. The insertions for scalar registers can still go at the BB top.

llvmbot · 2023-11-13T17:36:17Z

@llvm/pr-subscribers-llvm-regalloc

Author: Christudasan Devadasan (cdevadas)

Changes

We adjust the insertion point at the BB top for spills/copies during RA to ensure they are placed after the exec restore instructions required for the divergent control flow execution. This is, however, required only for the vector operations. The insertions for scalar registers can still go at the BB top.

Full diff: https://github.com/llvm/llvm-project/pull/72140.diff

12 Files Affected:

(modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+4-2)
(modified) llvm/include/llvm/CodeGen/TargetInstrInfo.h (+2-1)
(modified) llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp (+2-1)
(modified) llvm/lib/CodeGen/InlineSpiller.cpp (+1-1)
(modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+3-3)
(modified) llvm/lib/CodeGen/MachineSink.cpp (+1-1)
(modified) llvm/lib/CodeGen/RegAllocFast.cpp (+1-1)
(modified) llvm/lib/CodeGen/SplitKit.cpp (+4-2)
(modified) llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp (+2-2)
(modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+11-3)
(modified) llvm/lib/Target/AMDGPU/SIInstrInfo.h (+2-1)
(modified) llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll (+60-59)

diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 4b5336fac33ea46..2caefe80f352791 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -846,8 +846,10 @@ class MachineBasicBlock
 
   /// Return the first instruction in MBB after I that is not a PHI, label or
   /// debug.  This is the correct point to insert copies at the beginning of a
-  /// basic block.
-  iterator SkipPHIsLabelsAndDebug(iterator I, bool SkipPseudoOp = true);
+  /// basic block. The optional arg \p Reg would allow targets to apply some
+  /// additional checks while inserting copies/spills during RA.
+  iterator SkipPHIsLabelsAndDebug(iterator I, Register Reg = Register(),
+                                  bool SkipPseudoOp = true);
 
   /// Returns an iterator to the first terminator instruction of this basic
   /// block. If a terminator does not exist, it returns end().
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 8e7499ac626a747..0487fd644d0ddf3 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -1989,7 +1989,8 @@ class TargetInstrInfo : public MCInstrInfo {
   /// True if the instruction is bound to the top of its basic block and no
   /// other instructions shall be inserted before it. This can be implemented
   /// to prevent register allocator to insert spills before such instructions.
-  virtual bool isBasicBlockPrologue(const MachineInstr &MI) const {
+  virtual bool isBasicBlockPrologue(const MachineInstr &MI,
+                                    Register Reg) const {
     return false;
   }
 
diff --git a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
index 75504ef32250c52..4d668c53f7156b8 100644
--- a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
+++ b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
@@ -461,7 +461,8 @@ class StatepointState {
 
       if (EHPad && !RC.hasReload(Reg, RegToSlotIdx[Reg], EHPad)) {
         RC.recordReload(Reg, RegToSlotIdx[Reg], EHPad);
-        auto EHPadInsertPoint = EHPad->SkipPHIsLabelsAndDebug(EHPad->begin());
+        auto EHPadInsertPoint =
+            EHPad->SkipPHIsLabelsAndDebug(EHPad->begin(), Reg);
         insertReloadBefore(Reg, EHPadInsertPoint, EHPad);
         LLVM_DEBUG(dbgs() << "...also reload at EHPad "
                           << printMBBReference(*EHPad) << "\n");
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 71d58b2e9e18d7d..2740265f75340b5 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -469,7 +469,7 @@ bool InlineSpiller::hoistSpillInsideBB(LiveInterval &SpillLI,
   MachineBasicBlock *MBB = LIS.getMBBFromIndex(SrcVNI->def);
   MachineBasicBlock::iterator MII;
   if (SrcVNI->isPHIDef())
-    MII = MBB->SkipPHIsLabelsAndDebug(MBB->begin());
+    MII = MBB->SkipPHIsLabelsAndDebug(MBB->begin(), SrcReg);
   else {
     MachineInstr *DefMI = LIS.getInstructionFromIndex(SrcVNI->def);
     assert(DefMI && "Defining instruction disappeared");
diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index d9e22685faf5f5e..0c92d5d910dc14e 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -212,7 +212,7 @@ MachineBasicBlock::SkipPHIsAndLabels(MachineBasicBlock::iterator I) {
 
   iterator E = end();
   while (I != E && (I->isPHI() || I->isPosition() ||
-                    TII->isBasicBlockPrologue(*I)))
+                    TII->isBasicBlockPrologue(*I, Register())))
     ++I;
   // FIXME: This needs to change if we wish to bundle labels
   // inside the bundle.
@@ -223,13 +223,13 @@ MachineBasicBlock::SkipPHIsAndLabels(MachineBasicBlock::iterator I) {
 
 MachineBasicBlock::iterator
 MachineBasicBlock::SkipPHIsLabelsAndDebug(MachineBasicBlock::iterator I,
-                                          bool SkipPseudoOp) {
+                                          Register Reg, bool SkipPseudoOp) {
   const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();
 
   iterator E = end();
   while (I != E && (I->isPHI() || I->isPosition() || I->isDebugInstr() ||
                     (SkipPseudoOp && I->isPseudoProbe()) ||
-                    TII->isBasicBlockPrologue(*I)))
+                    TII->isBasicBlockPrologue(*I, Reg)))
     ++I;
   // FIXME: This needs to change if we wish to bundle labels / dbg_values
   // inside the bundle.
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index 9ed6b22eae972cc..1da6ccccbd540c4 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -291,7 +291,7 @@ static bool blockPrologueInterferes(const MachineBasicBlock *BB,
   for (MachineBasicBlock::const_iterator PI = BB->getFirstNonPHI(); PI != End;
        ++PI) {
     // Only check target defined prologue instructions
-    if (!TII->isBasicBlockPrologue(*PI))
+    if (!TII->isBasicBlockPrologue(*PI, Register()))
       continue;
     for (auto &MO : MI.operands()) {
       if (!MO.isReg())
diff --git a/llvm/lib/CodeGen/RegAllocFast.cpp b/llvm/lib/CodeGen/RegAllocFast.cpp
index 9f3c17e2799ceae..03f8c26ad2d0f8f 100644
--- a/llvm/lib/CodeGen/RegAllocFast.cpp
+++ b/llvm/lib/CodeGen/RegAllocFast.cpp
@@ -514,7 +514,7 @@ RegAllocFast::getMBBBeginInsertionPoint(
     }
 
     // Most reloads should be inserted after prolog instructions.
-    if (!TII->isBasicBlockPrologue(*I))
+    if (!TII->isBasicBlockPrologue(*I, Register()))
       break;
 
     // However if a prolog instruction reads a register that needs to be
diff --git a/llvm/lib/CodeGen/SplitKit.cpp b/llvm/lib/CodeGen/SplitKit.cpp
index 1664c304f643c3f..b1c862210932bc3 100644
--- a/llvm/lib/CodeGen/SplitKit.cpp
+++ b/llvm/lib/CodeGen/SplitKit.cpp
@@ -795,8 +795,10 @@ SlotIndex SplitEditor::leaveIntvAtTop(MachineBasicBlock &MBB) {
     return Start;
   }
 
-  VNInfo *VNI = defFromParent(0, ParentVNI, Start, MBB,
-                              MBB.SkipPHIsLabelsAndDebug(MBB.begin()));
+  unsigned RegIdx = 0;
+  Register Reg = LIS.getInterval(Edit->get(RegIdx)).reg();
+  VNInfo *VNI = defFromParent(RegIdx, ParentVNI, Start, MBB,
+                              MBB.SkipPHIsLabelsAndDebug(MBB.begin(), Reg));
   RegAssign.insert(Start, VNI->def, OpenIdx);
   LLVM_DEBUG(dump());
   return VNI->def;
diff --git a/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp b/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
index 86980ee851bb1fc..1cb0e96e0e22a2a 100644
--- a/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
@@ -424,7 +424,7 @@ static bool isReachable(const MachineInstr *From,
 static MachineBasicBlock::iterator
 getFirstNonPrologue(MachineBasicBlock *MBB, const TargetInstrInfo *TII) {
   MachineBasicBlock::iterator I = MBB->getFirstNonPHI();
-  while (I != MBB->end() && TII->isBasicBlockPrologue(*I))
+  while (I != MBB->end() && TII->isBasicBlockPrologue(*I, Register()))
     ++I;
 
   return I;
@@ -578,7 +578,7 @@ static bool hoistAndMergeSGPRInits(unsigned Reg,
       MachineBasicBlock::reverse_iterator B(BoundaryMI);
       // Check if B should actually be a boundary. If not set the previous
       // instruction as the boundary instead.
-      if (!TII->isBasicBlockPrologue(*B))
+      if (!TII->isBasicBlockPrologue(*B, Register()))
         B++;
 
       auto R = std::next(MI->getReverseIterator());
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 027b695c3bb1a74..09682a387b45a86 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -8474,16 +8474,24 @@ unsigned SIInstrInfo::getLiveRangeSplitOpcode(Register SrcReg,
   return AMDGPU::COPY;
 }
 
-bool SIInstrInfo::isBasicBlockPrologue(const MachineInstr &MI) const {
+bool SIInstrInfo::isBasicBlockPrologue(const MachineInstr &MI,
+                                       Register Reg) const {
   // We need to handle instructions which may be inserted during register
   // allocation to handle the prolog. The initial prolog instruction may have
   // been separated from the start of the block by spills and copies inserted
-  // needed by the prolog.
+  // needed by the prolog. However, the insertions for scalar registers can
+  // always be placed at the BB top as they are independent of the exec mask
+  // value.
   uint16_t Opc = MI.getOpcode();
+  const MachineFunction *MF = MI.getParent()->getParent();
+  const MachineRegisterInfo &MRI = MF->getRegInfo();
+  bool IsNullOrVectorRegister =
+      !Reg || !RI.isSGPRClass(RI.getRegClassForReg(MRI, Reg));
 
   // FIXME: Copies inserted in the block prolog for live-range split should also
   // be included.
-  return (isSpillOpcode(Opc) || (!MI.isTerminator() && Opc != AMDGPU::COPY &&
+  return IsNullOrVectorRegister &&
+         (isSpillOpcode(Opc) || (!MI.isTerminator() && Opc != AMDGPU::COPY &&
                                  MI.modifiesRegister(AMDGPU::EXEC, &RI)));
 }
 
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index 29f549fc29a3ce6..6834a661196ed75 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -1179,7 +1179,8 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
   unsigned getLiveRangeSplitOpcode(Register Reg,
                                    const MachineFunction &MF) const override;
 
-  bool isBasicBlockPrologue(const MachineInstr &MI) const override;
+  bool isBasicBlockPrologue(const MachineInstr &MI,
+                            Register Reg) const override;
 
   MachineInstr *createPHIDestinationCopy(MachineBasicBlock &MBB,
                                          MachineBasicBlock::iterator InsPt,
diff --git a/llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll b/llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll
index 8098304d134229d..ffbf00765adbe22 100644
--- a/llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll
+++ b/llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll
@@ -168,7 +168,6 @@ define void @main(i1 %arg) #0 {
 ; CHECK-NEXT:    s_mov_b64 vcc, vcc
 ; CHECK-NEXT:    s_cbranch_vccnz .LBB0_2
 ; CHECK-NEXT:  .LBB0_3: ; %Flow14
-; CHECK-NEXT:    s_or_saveexec_b64 s[20:21], s[26:27]
 ; CHECK-NEXT:    s_waitcnt lgkmcnt(0)
 ; CHECK-NEXT:    v_readlane_b32 s12, v5, 32
 ; CHECK-NEXT:    v_readlane_b32 s13, v5, 33
@@ -178,39 +177,39 @@ define void @main(i1 %arg) #0 {
 ; CHECK-NEXT:    v_readlane_b32 s17, v5, 37
 ; CHECK-NEXT:    v_readlane_b32 s18, v5, 38
 ; CHECK-NEXT:    v_readlane_b32 s19, v5, 39
-; CHECK-NEXT:    v_writelane_b32 v5, s4, 56
-; CHECK-NEXT:    v_writelane_b32 v5, s5, 57
-; CHECK-NEXT:    v_writelane_b32 v5, s6, 58
-; CHECK-NEXT:    v_writelane_b32 v5, s7, 59
-; CHECK-NEXT:    v_writelane_b32 v5, s8, 60
-; CHECK-NEXT:    v_writelane_b32 v5, s9, 61
-; CHECK-NEXT:    v_writelane_b32 v5, s10, 62
-; CHECK-NEXT:    v_writelane_b32 v5, s11, 63
-; CHECK-NEXT:    v_writelane_b32 v5, s52, 40
-; CHECK-NEXT:    v_writelane_b32 v5, s53, 41
-; CHECK-NEXT:    v_writelane_b32 v5, s54, 42
-; CHECK-NEXT:    v_writelane_b32 v5, s55, 43
-; CHECK-NEXT:    v_writelane_b32 v5, s56, 44
-; CHECK-NEXT:    v_writelane_b32 v5, s57, 45
-; CHECK-NEXT:    v_writelane_b32 v5, s58, 46
-; CHECK-NEXT:    v_writelane_b32 v5, s59, 47
-; CHECK-NEXT:    v_writelane_b32 v4, s12, 0
-; CHECK-NEXT:    v_writelane_b32 v5, s60, 48
-; CHECK-NEXT:    v_writelane_b32 v4, s13, 1
-; CHECK-NEXT:    v_writelane_b32 v5, s61, 49
-; CHECK-NEXT:    v_writelane_b32 v4, s14, 2
-; CHECK-NEXT:    v_writelane_b32 v5, s62, 50
-; CHECK-NEXT:    v_writelane_b32 v4, s15, 3
-; CHECK-NEXT:    v_writelane_b32 v5, s63, 51
-; CHECK-NEXT:    v_writelane_b32 v4, s16, 4
-; CHECK-NEXT:    v_writelane_b32 v5, s64, 52
-; CHECK-NEXT:    v_writelane_b32 v4, s17, 5
-; CHECK-NEXT:    v_writelane_b32 v5, s65, 53
-; CHECK-NEXT:    v_writelane_b32 v4, s18, 6
-; CHECK-NEXT:    v_writelane_b32 v5, s66, 54
-; CHECK-NEXT:    v_writelane_b32 v4, s19, 7
-; CHECK-NEXT:    v_writelane_b32 v5, s67, 55
-; CHECK-NEXT:    s_xor_b64 exec, exec, s[20:21]
+; CHECK-NEXT:    v_writelane_b32 v5, s4, 40
+; CHECK-NEXT:    v_writelane_b32 v5, s5, 41
+; CHECK-NEXT:    v_writelane_b32 v5, s6, 42
+; CHECK-NEXT:    v_writelane_b32 v5, s7, 43
+; CHECK-NEXT:    v_writelane_b32 v5, s8, 44
+; CHECK-NEXT:    v_writelane_b32 v5, s9, 45
+; CHECK-NEXT:    v_writelane_b32 v5, s10, 46
+; CHECK-NEXT:    v_writelane_b32 v5, s11, 47
+; CHECK-NEXT:    v_writelane_b32 v5, s12, 48
+; CHECK-NEXT:    v_writelane_b32 v5, s13, 49
+; CHECK-NEXT:    v_writelane_b32 v5, s14, 50
+; CHECK-NEXT:    v_writelane_b32 v5, s15, 51
+; CHECK-NEXT:    v_writelane_b32 v5, s16, 52
+; CHECK-NEXT:    v_writelane_b32 v5, s17, 53
+; CHECK-NEXT:    v_writelane_b32 v5, s18, 54
+; CHECK-NEXT:    v_writelane_b32 v5, s19, 55
+; CHECK-NEXT:    v_writelane_b32 v5, s52, 56
+; CHECK-NEXT:    v_writelane_b32 v4, s60, 0
+; CHECK-NEXT:    v_writelane_b32 v5, s53, 57
+; CHECK-NEXT:    v_writelane_b32 v4, s61, 1
+; CHECK-NEXT:    v_writelane_b32 v5, s54, 58
+; CHECK-NEXT:    v_writelane_b32 v4, s62, 2
+; CHECK-NEXT:    v_writelane_b32 v5, s55, 59
+; CHECK-NEXT:    v_writelane_b32 v4, s63, 3
+; CHECK-NEXT:    v_writelane_b32 v5, s56, 60
+; CHECK-NEXT:    v_writelane_b32 v4, s64, 4
+; CHECK-NEXT:    v_writelane_b32 v5, s57, 61
+; CHECK-NEXT:    v_writelane_b32 v4, s65, 5
+; CHECK-NEXT:    v_writelane_b32 v5, s58, 62
+; CHECK-NEXT:    v_writelane_b32 v4, s66, 6
+; CHECK-NEXT:    v_writelane_b32 v5, s59, 63
+; CHECK-NEXT:    v_writelane_b32 v4, s67, 7
+; CHECK-NEXT:    s_andn2_saveexec_b64 s[20:21], s[26:27]
 ; CHECK-NEXT:    s_cbranch_execz .LBB0_10
 ; CHECK-NEXT:  ; %bb.4: ; %bb32
 ; CHECK-NEXT:    s_and_saveexec_b64 s[8:9], s[24:25]
@@ -265,39 +264,35 @@ define void @main(i1 %arg) #0 {
 ; CHECK-NEXT:    s_waitcnt vmcnt(1)
 ; CHECK-NEXT:    buffer_store_dwordx4 v[2:5], off, s[8:11], 0
 ; CHECK-NEXT:  .LBB0_6: ; %Flow12
-; CHECK-NEXT:    s_andn2_saveexec_b64 s[4:5], s[22:23]
+; CHECK-NEXT:    s_or_saveexec_b64 s[4:5], s[22:23]
+; CHECK-NEXT:    v_readlane_b32 s52, v5, 40
+; CHECK-NEXT:    v_readlane_b32 s53, v5, 41
+; CHECK-NEXT:    v_readlane_b32 s54, v5, 42
+; CHECK-NEXT:    v_readlane_b32 s55, v5, 43
+; CHECK-NEXT:    v_readlane_b32 s56, v5, 44
+; CHECK-NEXT:    v_readlane_b32 s57, v5, 45
+; CHECK-NEXT:    v_readlane_b32 s58, v5, 46
+; CHECK-NEXT:    v_readlane_b32 s59, v5, 47
+; CHECK-NEXT:    v_readlane_b32 s60, v5, 48
+; CHECK-NEXT:    v_readlane_b32 s61, v5, 49
+; CHECK-NEXT:    v_readlane_b32 s62, v5, 50
+; CHECK-NEXT:    v_readlane_b32 s63, v5, 51
+; CHECK-NEXT:    v_readlane_b32 s64, v5, 52
+; CHECK-NEXT:    v_readlane_b32 s65, v5, 53
+; CHECK-NEXT:    v_readlane_b32 s66, v5, 54
+; CHECK-NEXT:    v_readlane_b32 s67, v5, 55
+; CHECK-NEXT:    s_xor_b64 exec, exec, s[4:5]
 ; CHECK-NEXT:    s_cbranch_execz .LBB0_9
 ; CHECK-NEXT:  ; %bb.7: ; %bb33.preheader
 ; CHECK-NEXT:    s_mov_b32 s8, 0
 ; CHECK-NEXT:    s_mov_b32 s6, s8
-; CHECK-NEXT:    v_readlane_b32 s36, v5, 40
 ; CHECK-NEXT:    s_mov_b32 s7, s8
 ; CHECK-NEXT:    v_mov_b32_e32 v2, s6
-; CHECK-NEXT:    v_readlane_b32 s37, v5, 41
+; CHECK-NEXT:    v_readlane_b32 s36, v5, 56
 ; CHECK-NEXT:    s_mov_b32 s9, s8
 ; CHECK-NEXT:    s_mov_b32 s10, s8
 ; CHECK-NEXT:    s_mov_b32 s11, s8
 ; CHECK-NEXT:    v_mov_b32_e32 v3, s7
-; CHECK-NEXT:    v_readlane_b32 s38, v5, 42
-; CHECK-NEXT:    v_readlane_b32 s39, v5, 43
-; CHECK-NEXT:    v_readlane_b32 s40, v5, 44
-; CHECK-NEXT:    v_readlane_b32 s41, v5, 45
-; CHECK-NEXT:    v_readlane_b32 s42, v5, 46
-; CHECK-NEXT:    v_readlane_b32 s43, v5, 47
-; CHECK-NEXT:    v_readlane_b32 s44, v5, 48
-; CHECK-NEXT:    v_readlane_b32 s45, v5, 49
-; CHECK-NEXT:    v_readlane_b32 s46, v5, 50
-; CHECK-NEXT:    v_readlane_b32 s47, v5, 51
-; CHECK-NEXT:    v_readlane_b32 s48, v5, 52
-; CHECK-NEXT:    v_readlane_b32 s49, v5, 53
-; CHECK-NEXT:    v_readlane_b32 s50, v5, 54
-; CHECK-NEXT:    v_readlane_b32 s51, v5, 55
-; CHECK-NEXT:    s_mov_b64 s[12:13], s[36:37]
-; CHECK-NEXT:    s_mov_b64 s[14:15], s[38:39]
-; CHECK-NEXT:    s_mov_b64 s[16:17], s[40:41]
-; CHECK-NEXT:    s_mov_b64 s[18:19], s[42:43]
-; CHECK-NEXT:    image_sample_lz v6, v[2:3], s[36:43], s[8:11] dmask:0x1
-; CHECK-NEXT:    v_readlane_b32 s36, v5, 56
 ; CHECK-NEXT:    v_readlane_b32 s37, v5, 57
 ; CHECK-NEXT:    v_readlane_b32 s38, v5, 58
 ; CHECK-NEXT:    v_readlane_b32 s39, v5, 59
@@ -305,19 +300,25 @@ define void @main(i1 %arg) #0 {
 ; CHECK-NEXT:    v_readlane_b32 s41, v5, 61
 ; CHECK-NEXT:    v_readlane_b32 s42, v5, 62
 ; CHECK-NEXT:    v_readlane_b32 s43, v5, 63
+; CHECK-NEXT:    s_nop 4
+; CHECK-NEXT:    image_sample_lz v6, v[2:3], s[36:43], s[8:11] dmask:0x1
+; CHECK-NEXT:    image_sample_lz v7, v[2:3], s[52:59], s[8:11] dmask:0x1
 ; CHECK-NEXT:    ; kill: killed $vgpr2_vgpr3
+; CHECK-NEXT:    s_mov_b64 s[12:13], s[36:37]
 ; CHECK-NEXT:    s_and_b64 vcc, exec, 0
 ; CHECK-NEXT:    v_readlane_b32 s44, v4, 0
 ; CHECK-NEXT:    v_readlane_b32 s45, v4, 1
 ; CHECK-NEXT:    v_readlane_b32 s46, v4, 2
 ; CHECK-NEXT:    v_readlane_b32 s47, v4, 3
-; CHECK-NEXT:    image_sample_lz v7, v[2:3], s[36:43], s[8:11] dmask:0x1
 ; CHECK-NEXT:    v_readlane_b32 s48, v4, 4
 ; CHECK-NEXT:    v_readlane_b32 s49, v4, 5
 ; CHECK-NEXT:    v_readlane_b32 s50, v4, 6
 ; CHECK-NEXT:    v_readlane_b32 s51, v4, 7
+; CHECK-NEXT:    s_mov_b64 s[14:15], s[38:39]
+; CHECK-NEXT:    s_mov_b64 s[16:17], s[40:41]
+; CHECK-NEXT:    s_mov_b64 s[18:19], s[42:43]
 ; CHECK-NEXT:    ; kill: killed $sgpr12_sgpr13_sgpr14_sgpr15_sgpr16_sgpr17_sgpr18_sgpr19
-; CHECK-NEXT:    ; kill: killed $sgpr36_sgpr37_sgpr38_sgpr39_sgpr40_sgpr41_sgpr42_sgpr43
+; CHECK-NEXT:    ; kill: killed $sgpr52_sgpr53_sgpr54_sgpr55_sgpr56_sgpr57_sgpr58_sgpr59
 ; CHECK-NEXT:    ; kill: killed $sgpr8_sgpr9_sgpr10 killed $sgpr11
 ; CHECK-NEXT:    s_waitcnt vmcnt(0)
 ; CHECK-NEXT:    v_sub_f32_e32 v2, v7, v6

llvmbot · 2023-11-13T17:36:17Z

@llvm/pr-subscribers-backend-amdgpu

Author: Christudasan Devadasan (cdevadas)

Changes

We adjust the insertion point at the BB top for spills/copies during RA to ensure they are placed after the exec restore instructions required for the divergent control flow execution. This is, however, required only for the vector operations. The insertions for scalar registers can still go at the BB top.

Full diff: https://github.com/llvm/llvm-project/pull/72140.diff

12 Files Affected:

(modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+4-2)
(modified) llvm/include/llvm/CodeGen/TargetInstrInfo.h (+2-1)
(modified) llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp (+2-1)
(modified) llvm/lib/CodeGen/InlineSpiller.cpp (+1-1)
(modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+3-3)
(modified) llvm/lib/CodeGen/MachineSink.cpp (+1-1)
(modified) llvm/lib/CodeGen/RegAllocFast.cpp (+1-1)
(modified) llvm/lib/CodeGen/SplitKit.cpp (+4-2)
(modified) llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp (+2-2)
(modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+11-3)
(modified) llvm/lib/Target/AMDGPU/SIInstrInfo.h (+2-1)
(modified) llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll (+60-59)

diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 4b5336fac33ea46..2caefe80f352791 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -846,8 +846,10 @@ class MachineBasicBlock
 
   /// Return the first instruction in MBB after I that is not a PHI, label or
   /// debug.  This is the correct point to insert copies at the beginning of a
-  /// basic block.
-  iterator SkipPHIsLabelsAndDebug(iterator I, bool SkipPseudoOp = true);
+  /// basic block. The optional arg \p Reg would allow targets to apply some
+  /// additional checks while inserting copies/spills during RA.
+  iterator SkipPHIsLabelsAndDebug(iterator I, Register Reg = Register(),
+                                  bool SkipPseudoOp = true);
 
   /// Returns an iterator to the first terminator instruction of this basic
   /// block. If a terminator does not exist, it returns end().
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 8e7499ac626a747..0487fd644d0ddf3 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -1989,7 +1989,8 @@ class TargetInstrInfo : public MCInstrInfo {
   /// True if the instruction is bound to the top of its basic block and no
   /// other instructions shall be inserted before it. This can be implemented
   /// to prevent register allocator to insert spills before such instructions.
-  virtual bool isBasicBlockPrologue(const MachineInstr &MI) const {
+  virtual bool isBasicBlockPrologue(const MachineInstr &MI,
+                                    Register Reg) const {
     return false;
   }
 
diff --git a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
index 75504ef32250c52..4d668c53f7156b8 100644
--- a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
+++ b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
@@ -461,7 +461,8 @@ class StatepointState {
 
       if (EHPad && !RC.hasReload(Reg, RegToSlotIdx[Reg], EHPad)) {
         RC.recordReload(Reg, RegToSlotIdx[Reg], EHPad);
-        auto EHPadInsertPoint = EHPad->SkipPHIsLabelsAndDebug(EHPad->begin());
+        auto EHPadInsertPoint =
+            EHPad->SkipPHIsLabelsAndDebug(EHPad->begin(), Reg);
         insertReloadBefore(Reg, EHPadInsertPoint, EHPad);
         LLVM_DEBUG(dbgs() << "...also reload at EHPad "
                           << printMBBReference(*EHPad) << "\n");
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 71d58b2e9e18d7d..2740265f75340b5 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -469,7 +469,7 @@ bool InlineSpiller::hoistSpillInsideBB(LiveInterval &SpillLI,
   MachineBasicBlock *MBB = LIS.getMBBFromIndex(SrcVNI->def);
   MachineBasicBlock::iterator MII;
   if (SrcVNI->isPHIDef())
-    MII = MBB->SkipPHIsLabelsAndDebug(MBB->begin());
+    MII = MBB->SkipPHIsLabelsAndDebug(MBB->begin(), SrcReg);
   else {
     MachineInstr *DefMI = LIS.getInstructionFromIndex(SrcVNI->def);
     assert(DefMI && "Defining instruction disappeared");
diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index d9e22685faf5f5e..0c92d5d910dc14e 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -212,7 +212,7 @@ MachineBasicBlock::SkipPHIsAndLabels(MachineBasicBlock::iterator I) {
 
   iterator E = end();
   while (I != E && (I->isPHI() || I->isPosition() ||
-                    TII->isBasicBlockPrologue(*I)))
+                    TII->isBasicBlockPrologue(*I, Register())))
     ++I;
   // FIXME: This needs to change if we wish to bundle labels
   // inside the bundle.
@@ -223,13 +223,13 @@ MachineBasicBlock::SkipPHIsAndLabels(MachineBasicBlock::iterator I) {
 
 MachineBasicBlock::iterator
 MachineBasicBlock::SkipPHIsLabelsAndDebug(MachineBasicBlock::iterator I,
-                                          bool SkipPseudoOp) {
+                                          Register Reg, bool SkipPseudoOp) {
   const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();
 
   iterator E = end();
   while (I != E && (I->isPHI() || I->isPosition() || I->isDebugInstr() ||
                     (SkipPseudoOp && I->isPseudoProbe()) ||
-                    TII->isBasicBlockPrologue(*I)))
+                    TII->isBasicBlockPrologue(*I, Reg)))
     ++I;
   // FIXME: This needs to change if we wish to bundle labels / dbg_values
   // inside the bundle.
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index 9ed6b22eae972cc..1da6ccccbd540c4 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -291,7 +291,7 @@ static bool blockPrologueInterferes(const MachineBasicBlock *BB,
   for (MachineBasicBlock::const_iterator PI = BB->getFirstNonPHI(); PI != End;
        ++PI) {
     // Only check target defined prologue instructions
-    if (!TII->isBasicBlockPrologue(*PI))
+    if (!TII->isBasicBlockPrologue(*PI, Register()))
       continue;
     for (auto &MO : MI.operands()) {
       if (!MO.isReg())
diff --git a/llvm/lib/CodeGen/RegAllocFast.cpp b/llvm/lib/CodeGen/RegAllocFast.cpp
index 9f3c17e2799ceae..03f8c26ad2d0f8f 100644
--- a/llvm/lib/CodeGen/RegAllocFast.cpp
+++ b/llvm/lib/CodeGen/RegAllocFast.cpp
@@ -514,7 +514,7 @@ RegAllocFast::getMBBBeginInsertionPoint(
     }
 
     // Most reloads should be inserted after prolog instructions.
-    if (!TII->isBasicBlockPrologue(*I))
+    if (!TII->isBasicBlockPrologue(*I, Register()))
       break;
 
     // However if a prolog instruction reads a register that needs to be
diff --git a/llvm/lib/CodeGen/SplitKit.cpp b/llvm/lib/CodeGen/SplitKit.cpp
index 1664c304f643c3f..b1c862210932bc3 100644
--- a/llvm/lib/CodeGen/SplitKit.cpp
+++ b/llvm/lib/CodeGen/SplitKit.cpp
@@ -795,8 +795,10 @@ SlotIndex SplitEditor::leaveIntvAtTop(MachineBasicBlock &MBB) {
     return Start;
   }
 
-  VNInfo *VNI = defFromParent(0, ParentVNI, Start, MBB,
-                              MBB.SkipPHIsLabelsAndDebug(MBB.begin()));
+  unsigned RegIdx = 0;
+  Register Reg = LIS.getInterval(Edit->get(RegIdx)).reg();
+  VNInfo *VNI = defFromParent(RegIdx, ParentVNI, Start, MBB,
+                              MBB.SkipPHIsLabelsAndDebug(MBB.begin(), Reg));
   RegAssign.insert(Start, VNI->def, OpenIdx);
   LLVM_DEBUG(dump());
   return VNI->def;
diff --git a/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp b/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
index 86980ee851bb1fc..1cb0e96e0e22a2a 100644
--- a/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
@@ -424,7 +424,7 @@ static bool isReachable(const MachineInstr *From,
 static MachineBasicBlock::iterator
 getFirstNonPrologue(MachineBasicBlock *MBB, const TargetInstrInfo *TII) {
   MachineBasicBlock::iterator I = MBB->getFirstNonPHI();
-  while (I != MBB->end() && TII->isBasicBlockPrologue(*I))
+  while (I != MBB->end() && TII->isBasicBlockPrologue(*I, Register()))
     ++I;
 
   return I;
@@ -578,7 +578,7 @@ static bool hoistAndMergeSGPRInits(unsigned Reg,
       MachineBasicBlock::reverse_iterator B(BoundaryMI);
       // Check if B should actually be a boundary. If not set the previous
       // instruction as the boundary instead.
-      if (!TII->isBasicBlockPrologue(*B))
+      if (!TII->isBasicBlockPrologue(*B, Register()))
         B++;
 
       auto R = std::next(MI->getReverseIterator());
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 027b695c3bb1a74..09682a387b45a86 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -8474,16 +8474,24 @@ unsigned SIInstrInfo::getLiveRangeSplitOpcode(Register SrcReg,
   return AMDGPU::COPY;
 }
 
-bool SIInstrInfo::isBasicBlockPrologue(const MachineInstr &MI) const {
+bool SIInstrInfo::isBasicBlockPrologue(const MachineInstr &MI,
+                                       Register Reg) const {
   // We need to handle instructions which may be inserted during register
   // allocation to handle the prolog. The initial prolog instruction may have
   // been separated from the start of the block by spills and copies inserted
-  // needed by the prolog.
+  // needed by the prolog. However, the insertions for scalar registers can
+  // always be placed at the BB top as they are independent of the exec mask
+  // value.
   uint16_t Opc = MI.getOpcode();
+  const MachineFunction *MF = MI.getParent()->getParent();
+  const MachineRegisterInfo &MRI = MF->getRegInfo();
+  bool IsNullOrVectorRegister =
+      !Reg || !RI.isSGPRClass(RI.getRegClassForReg(MRI, Reg));
 
   // FIXME: Copies inserted in the block prolog for live-range split should also
   // be included.
-  return (isSpillOpcode(Opc) || (!MI.isTerminator() && Opc != AMDGPU::COPY &&
+  return IsNullOrVectorRegister &&
+         (isSpillOpcode(Opc) || (!MI.isTerminator() && Opc != AMDGPU::COPY &&
                                  MI.modifiesRegister(AMDGPU::EXEC, &RI)));
 }
 
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index 29f549fc29a3ce6..6834a661196ed75 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -1179,7 +1179,8 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
   unsigned getLiveRangeSplitOpcode(Register Reg,
                                    const MachineFunction &MF) const override;
 
-  bool isBasicBlockPrologue(const MachineInstr &MI) const override;
+  bool isBasicBlockPrologue(const MachineInstr &MI,
+                            Register Reg) const override;
 
   MachineInstr *createPHIDestinationCopy(MachineBasicBlock &MBB,
                                          MachineBasicBlock::iterator InsPt,
diff --git a/llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll b/llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll
index 8098304d134229d..ffbf00765adbe22 100644
--- a/llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll
+++ b/llvm/test/CodeGen/AMDGPU/identical-subrange-spill-infloop.ll
@@ -168,7 +168,6 @@ define void @main(i1 %arg) #0 {
 ; CHECK-NEXT:    s_mov_b64 vcc, vcc
 ; CHECK-NEXT:    s_cbranch_vccnz .LBB0_2
 ; CHECK-NEXT:  .LBB0_3: ; %Flow14
-; CHECK-NEXT:    s_or_saveexec_b64 s[20:21], s[26:27]
 ; CHECK-NEXT:    s_waitcnt lgkmcnt(0)
 ; CHECK-NEXT:    v_readlane_b32 s12, v5, 32
 ; CHECK-NEXT:    v_readlane_b32 s13, v5, 33
@@ -178,39 +177,39 @@ define void @main(i1 %arg) #0 {
 ; CHECK-NEXT:    v_readlane_b32 s17, v5, 37
 ; CHECK-NEXT:    v_readlane_b32 s18, v5, 38
 ; CHECK-NEXT:    v_readlane_b32 s19, v5, 39
-; CHECK-NEXT:    v_writelane_b32 v5, s4, 56
-; CHECK-NEXT:    v_writelane_b32 v5, s5, 57
-; CHECK-NEXT:    v_writelane_b32 v5, s6, 58
-; CHECK-NEXT:    v_writelane_b32 v5, s7, 59
-; CHECK-NEXT:    v_writelane_b32 v5, s8, 60
-; CHECK-NEXT:    v_writelane_b32 v5, s9, 61
-; CHECK-NEXT:    v_writelane_b32 v5, s10, 62
-; CHECK-NEXT:    v_writelane_b32 v5, s11, 63
-; CHECK-NEXT:    v_writelane_b32 v5, s52, 40
-; CHECK-NEXT:    v_writelane_b32 v5, s53, 41
-; CHECK-NEXT:    v_writelane_b32 v5, s54, 42
-; CHECK-NEXT:    v_writelane_b32 v5, s55, 43
-; CHECK-NEXT:    v_writelane_b32 v5, s56, 44
-; CHECK-NEXT:    v_writelane_b32 v5, s57, 45
-; CHECK-NEXT:    v_writelane_b32 v5, s58, 46
-; CHECK-NEXT:    v_writelane_b32 v5, s59, 47
-; CHECK-NEXT:    v_writelane_b32 v4, s12, 0
-; CHECK-NEXT:    v_writelane_b32 v5, s60, 48
-; CHECK-NEXT:    v_writelane_b32 v4, s13, 1
-; CHECK-NEXT:    v_writelane_b32 v5, s61, 49
-; CHECK-NEXT:    v_writelane_b32 v4, s14, 2
-; CHECK-NEXT:    v_writelane_b32 v5, s62, 50
-; CHECK-NEXT:    v_writelane_b32 v4, s15, 3
-; CHECK-NEXT:    v_writelane_b32 v5, s63, 51
-; CHECK-NEXT:    v_writelane_b32 v4, s16, 4
-; CHECK-NEXT:    v_writelane_b32 v5, s64, 52
-; CHECK-NEXT:    v_writelane_b32 v4, s17, 5
-; CHECK-NEXT:    v_writelane_b32 v5, s65, 53
-; CHECK-NEXT:    v_writelane_b32 v4, s18, 6
-; CHECK-NEXT:    v_writelane_b32 v5, s66, 54
-; CHECK-NEXT:    v_writelane_b32 v4, s19, 7
-; CHECK-NEXT:    v_writelane_b32 v5, s67, 55
-; CHECK-NEXT:    s_xor_b64 exec, exec, s[20:21]
+; CHECK-NEXT:    v_writelane_b32 v5, s4, 40
+; CHECK-NEXT:    v_writelane_b32 v5, s5, 41
+; CHECK-NEXT:    v_writelane_b32 v5, s6, 42
+; CHECK-NEXT:    v_writelane_b32 v5, s7, 43
+; CHECK-NEXT:    v_writelane_b32 v5, s8, 44
+; CHECK-NEXT:    v_writelane_b32 v5, s9, 45
+; CHECK-NEXT:    v_writelane_b32 v5, s10, 46
+; CHECK-NEXT:    v_writelane_b32 v5, s11, 47
+; CHECK-NEXT:    v_writelane_b32 v5, s12, 48
+; CHECK-NEXT:    v_writelane_b32 v5, s13, 49
+; CHECK-NEXT:    v_writelane_b32 v5, s14, 50
+; CHECK-NEXT:    v_writelane_b32 v5, s15, 51
+; CHECK-NEXT:    v_writelane_b32 v5, s16, 52
+; CHECK-NEXT:    v_writelane_b32 v5, s17, 53
+; CHECK-NEXT:    v_writelane_b32 v5, s18, 54
+; CHECK-NEXT:    v_writelane_b32 v5, s19, 55
+; CHECK-NEXT:    v_writelane_b32 v5, s52, 56
+; CHECK-NEXT:    v_writelane_b32 v4, s60, 0
+; CHECK-NEXT:    v_writelane_b32 v5, s53, 57
+; CHECK-NEXT:    v_writelane_b32 v4, s61, 1
+; CHECK-NEXT:    v_writelane_b32 v5, s54, 58
+; CHECK-NEXT:    v_writelane_b32 v4, s62, 2
+; CHECK-NEXT:    v_writelane_b32 v5, s55, 59
+; CHECK-NEXT:    v_writelane_b32 v4, s63, 3
+; CHECK-NEXT:    v_writelane_b32 v5, s56, 60
+; CHECK-NEXT:    v_writelane_b32 v4, s64, 4
+; CHECK-NEXT:    v_writelane_b32 v5, s57, 61
+; CHECK-NEXT:    v_writelane_b32 v4, s65, 5
+; CHECK-NEXT:    v_writelane_b32 v5, s58, 62
+; CHECK-NEXT:    v_writelane_b32 v4, s66, 6
+; CHECK-NEXT:    v_writelane_b32 v5, s59, 63
+; CHECK-NEXT:    v_writelane_b32 v4, s67, 7
+; CHECK-NEXT:    s_andn2_saveexec_b64 s[20:21], s[26:27]
 ; CHECK-NEXT:    s_cbranch_execz .LBB0_10
 ; CHECK-NEXT:  ; %bb.4: ; %bb32
 ; CHECK-NEXT:    s_and_saveexec_b64 s[8:9], s[24:25]
@@ -265,39 +264,35 @@ define void @main(i1 %arg) #0 {
 ; CHECK-NEXT:    s_waitcnt vmcnt(1)
 ; CHECK-NEXT:    buffer_store_dwordx4 v[2:5], off, s[8:11], 0
 ; CHECK-NEXT:  .LBB0_6: ; %Flow12
-; CHECK-NEXT:    s_andn2_saveexec_b64 s[4:5], s[22:23]
+; CHECK-NEXT:    s_or_saveexec_b64 s[4:5], s[22:23]
+; CHECK-NEXT:    v_readlane_b32 s52, v5, 40
+; CHECK-NEXT:    v_readlane_b32 s53, v5, 41
+; CHECK-NEXT:    v_readlane_b32 s54, v5, 42
+; CHECK-NEXT:    v_readlane_b32 s55, v5, 43
+; CHECK-NEXT:    v_readlane_b32 s56, v5, 44
+; CHECK-NEXT:    v_readlane_b32 s57, v5, 45
+; CHECK-NEXT:    v_readlane_b32 s58, v5, 46
+; CHECK-NEXT:    v_readlane_b32 s59, v5, 47
+; CHECK-NEXT:    v_readlane_b32 s60, v5, 48
+; CHECK-NEXT:    v_readlane_b32 s61, v5, 49
+; CHECK-NEXT:    v_readlane_b32 s62, v5, 50
+; CHECK-NEXT:    v_readlane_b32 s63, v5, 51
+; CHECK-NEXT:    v_readlane_b32 s64, v5, 52
+; CHECK-NEXT:    v_readlane_b32 s65, v5, 53
+; CHECK-NEXT:    v_readlane_b32 s66, v5, 54
+; CHECK-NEXT:    v_readlane_b32 s67, v5, 55
+; CHECK-NEXT:    s_xor_b64 exec, exec, s[4:5]
 ; CHECK-NEXT:    s_cbranch_execz .LBB0_9
 ; CHECK-NEXT:  ; %bb.7: ; %bb33.preheader
 ; CHECK-NEXT:    s_mov_b32 s8, 0
 ; CHECK-NEXT:    s_mov_b32 s6, s8
-; CHECK-NEXT:    v_readlane_b32 s36, v5, 40
 ; CHECK-NEXT:    s_mov_b32 s7, s8
 ; CHECK-NEXT:    v_mov_b32_e32 v2, s6
-; CHECK-NEXT:    v_readlane_b32 s37, v5, 41
+; CHECK-NEXT:    v_readlane_b32 s36, v5, 56
 ; CHECK-NEXT:    s_mov_b32 s9, s8
 ; CHECK-NEXT:    s_mov_b32 s10, s8
 ; CHECK-NEXT:    s_mov_b32 s11, s8
 ; CHECK-NEXT:    v_mov_b32_e32 v3, s7
-; CHECK-NEXT:    v_readlane_b32 s38, v5, 42
-; CHECK-NEXT:    v_readlane_b32 s39, v5, 43
-; CHECK-NEXT:    v_readlane_b32 s40, v5, 44
-; CHECK-NEXT:    v_readlane_b32 s41, v5, 45
-; CHECK-NEXT:    v_readlane_b32 s42, v5, 46
-; CHECK-NEXT:    v_readlane_b32 s43, v5, 47
-; CHECK-NEXT:    v_readlane_b32 s44, v5, 48
-; CHECK-NEXT:    v_readlane_b32 s45, v5, 49
-; CHECK-NEXT:    v_readlane_b32 s46, v5, 50
-; CHECK-NEXT:    v_readlane_b32 s47, v5, 51
-; CHECK-NEXT:    v_readlane_b32 s48, v5, 52
-; CHECK-NEXT:    v_readlane_b32 s49, v5, 53
-; CHECK-NEXT:    v_readlane_b32 s50, v5, 54
-; CHECK-NEXT:    v_readlane_b32 s51, v5, 55
-; CHECK-NEXT:    s_mov_b64 s[12:13], s[36:37]
-; CHECK-NEXT:    s_mov_b64 s[14:15], s[38:39]
-; CHECK-NEXT:    s_mov_b64 s[16:17], s[40:41]
-; CHECK-NEXT:    s_mov_b64 s[18:19], s[42:43]
-; CHECK-NEXT:    image_sample_lz v6, v[2:3], s[36:43], s[8:11] dmask:0x1
-; CHECK-NEXT:    v_readlane_b32 s36, v5, 56
 ; CHECK-NEXT:    v_readlane_b32 s37, v5, 57
 ; CHECK-NEXT:    v_readlane_b32 s38, v5, 58
 ; CHECK-NEXT:    v_readlane_b32 s39, v5, 59
@@ -305,19 +300,25 @@ define void @main(i1 %arg) #0 {
 ; CHECK-NEXT:    v_readlane_b32 s41, v5, 61
 ; CHECK-NEXT:    v_readlane_b32 s42, v5, 62
 ; CHECK-NEXT:    v_readlane_b32 s43, v5, 63
+; CHECK-NEXT:    s_nop 4
+; CHECK-NEXT:    image_sample_lz v6, v[2:3], s[36:43], s[8:11] dmask:0x1
+; CHECK-NEXT:    image_sample_lz v7, v[2:3], s[52:59], s[8:11] dmask:0x1
 ; CHECK-NEXT:    ; kill: killed $vgpr2_vgpr3
+; CHECK-NEXT:    s_mov_b64 s[12:13], s[36:37]
 ; CHECK-NEXT:    s_and_b64 vcc, exec, 0
 ; CHECK-NEXT:    v_readlane_b32 s44, v4, 0
 ; CHECK-NEXT:    v_readlane_b32 s45, v4, 1
 ; CHECK-NEXT:    v_readlane_b32 s46, v4, 2
 ; CHECK-NEXT:    v_readlane_b32 s47, v4, 3
-; CHECK-NEXT:    image_sample_lz v7, v[2:3], s[36:43], s[8:11] dmask:0x1
 ; CHECK-NEXT:    v_readlane_b32 s48, v4, 4
 ; CHECK-NEXT:    v_readlane_b32 s49, v4, 5
 ; CHECK-NEXT:    v_readlane_b32 s50, v4, 6
 ; CHECK-NEXT:    v_readlane_b32 s51, v4, 7
+; CHECK-NEXT:    s_mov_b64 s[14:15], s[38:39]
+; CHECK-NEXT:    s_mov_b64 s[16:17], s[40:41]
+; CHECK-NEXT:    s_mov_b64 s[18:19], s[42:43]
 ; CHECK-NEXT:    ; kill: killed $sgpr12_sgpr13_sgpr14_sgpr15_sgpr16_sgpr17_sgpr18_sgpr19
-; CHECK-NEXT:    ; kill: killed $sgpr36_sgpr37_sgpr38_sgpr39_sgpr40_sgpr41_sgpr42_sgpr43
+; CHECK-NEXT:    ; kill: killed $sgpr52_sgpr53_sgpr54_sgpr55_sgpr56_sgpr57_sgpr58_sgpr59
 ; CHECK-NEXT:    ; kill: killed $sgpr8_sgpr9_sgpr10 killed $sgpr11
 ; CHECK-NEXT:    s_waitcnt vmcnt(0)
 ; CHECK-NEXT:    v_sub_f32_e32 v2, v7, v6

rampitec

You also need a specific small test for the situation.

rampitec · 2023-11-13T19:15:33Z

llvm/include/llvm/CodeGen/MachineBasicBlock.h

@@ -846,8 +846,10 @@ class MachineBasicBlock

  /// Return the first instruction in MBB after I that is not a PHI, label or
  /// debug.  This is the correct point to insert copies at the beginning of a
-  /// basic block.
-  iterator SkipPHIsLabelsAndDebug(iterator I, bool SkipPseudoOp = true);
+  /// basic block. The optional arg \p Reg would allow targets to apply some


The description of Reg does not seem to describe anything. What is this register?

+1 on the description not being descriptive enough.

rampitec · 2023-11-13T19:16:12Z

llvm/include/llvm/CodeGen/TargetInstrInfo.h

@@ -1989,7 +1989,8 @@ class TargetInstrInfo : public MCInstrInfo {
  /// True if the instruction is bound to the top of its basic block and no
  /// other instructions shall be inserted before it. This can be implemented
  /// to prevent register allocator to insert spills before such instructions.


Reg needs to be described.

rampitec · 2023-11-13T19:21:01Z

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

-  // needed by the prolog.
+  // needed by the prolog. However, the insertions for scalar registers can
+  // always be placed at the BB top as they are independent of the exec mask
+  // value.
  uint16_t Opc = MI.getOpcode();


You can avoid executing this for empty Reg.

Still the whole block can be avoided if Reg is unset.

llvm/include/llvm/CodeGen/TargetInstrInfo.h

We adjust the insertion point at the BB top for spills/copies during RA to ensure they are placed after the exec restore instructions required for the divergent control flow execution. This is, however, required only for the vector operations. The insertions for scalar registers can still go at the BB top.

cdevadas · 2023-11-14T21:23:45Z

Lit test precheckin + rebase + comments addressed.

rampitec · 2023-11-14T21:50:16Z

llvm/include/llvm/CodeGen/MachineBasicBlock.h

-  /// basic block.
-  iterator SkipPHIsLabelsAndDebug(iterator I, bool SkipPseudoOp = true);
+  /// basic block. \p Reg is an optional argument passed during register
+  /// allocator to have additional target specific checks for its spill/copy


allocator -> allocation. I also believe it shall be spill/restore/copy.

This is still a roundabout way of saying what the register value is. Say something that expresses this is the register being defined for a spill/split

I'm thinking this should be a differently named function, specifically for the spill insert point

rampitec · 2023-11-14T21:54:03Z

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

-  // needed by the prolog.
+  // needed by the prolog. However, the insertions for scalar registers can
+  // always be placed at the BB top as they are independent of the exec mask
+  // value.
  uint16_t Opc = MI.getOpcode();


Still the whole block can be avoided if Reg is unset.

cdevadas · 2023-11-15T18:57:38Z

Review comments addressed.

rampitec

LGTM with a small nit.

rampitec · 2023-11-15T19:06:12Z

llvm/include/llvm/CodeGen/MachineBasicBlock.h

@@ -846,8 +846,10 @@ class MachineBasicBlock

  /// Return the first instruction in MBB after I that is not a PHI, label or
  /// debug.  This is the correct point to insert copies at the beginning of a
-  /// basic block.
-  iterator SkipPHIsLabelsAndDebug(iterator I, bool SkipPseudoOp = true);
+  /// basic block. \p Reg is the register being defined for a spill/split during


Actually spill does not define a register, it kills it. Restore defines register.

rampitec

LGTM

…2140) We adjust the insertion point at the BB top for spills/copies during RA to ensure they are placed after the exec restore instructions required for the divergent control flow execution. This is, however, required only for the vector operations. The insertions for scalar registers can still go to the BB top.

Change-Id: I8f5e2bc14ab9ae77d0c3a4571549eaff02bd9cf9

…2140) We adjust the insertion point at the BB top for spills/copies during RA to ensure they are placed after the exec restore instructions required for the divergent control flow execution. This is, however, required only for the vector operations. The insertions for scalar registers can still go to the BB top. Change-Id: I0ee60b84c53c73d65d8bc9b6fdfc0bcb1e86c4fe

…llvm#72140)" This reverts commit ce7fd49. Change-Id: Iafb1f24fad78c477d3168a69cd8f9ccde6bc5038

This reverts commit 8f7e9f3. Change-Id: Ibfb2589ae430d99ec16e482bad5f585f5af100b5

…llvm#72140)" This reverts commit ce7fd49. Change-Id: Iafb1f24fad78c477d3168a69cd8f9ccde6bc5038

This reverts commit 8f7e9f3. Change-Id: Ibfb2589ae430d99ec16e482bad5f585f5af100b5

PRs llvm#69924 and llvm#72140 modified SIInstrInfo::isBasicBlockPrologue to skip over EXEC modifications and spills when allocating VGPRs. But treating VGPR spills as part of the prologue can confuse the register allocator as in llvm#109294, so restrict it to SGPR spills, which were inserted during SGPR allocation which is done in an earlier pass. Fixes: llvm#109294 Fixes: SWDEV-485841

…gue (#109439) PRs #69924 and #72140 modified SIInstrInfo::isBasicBlockPrologue to skip over EXEC modifications and spills when allocating VGPRs. But treating VGPR spills as part of the prologue can confuse the register allocator as in #109294, so restrict it to SGPR spills, which were inserted during SGPR allocation which is done in an earlier pass. Fixes: #109294 Fixes: SWDEV-485841

…gue (llvm#109439) PRs llvm#69924 and llvm#72140 modified SIInstrInfo::isBasicBlockPrologue to skip over EXEC modifications and spills when allocating VGPRs. But treating VGPR spills as part of the prologue can confuse the register allocator as in llvm#109294, so restrict it to SGPR spills, which were inserted during SGPR allocation which is done in an earlier pass. Fixes: llvm#109294 Fixes: SWDEV-485841

This reverts commit b633543. Change-Id: I4a7a15860efda59187e1daefd31eca806909c762

…e BB top (llvm#72140)"" This reverts commit 675ba5e. Change-Id: I19b8d08af0dbea3d08d3ff310f90cad11fb80351

PRs llvm#69924 and llvm#72140 modified SIInstrInfo::isBasicBlockPrologue to skip over EXEC modifications and spills when allocating VGPRs. But treating VGPR spills as part of the prologue can confuse the register allocator as in llvm#109294, so restrict it to SGPR spills, which were inserted during SGPR allocation which is done in an earlier pass. Fixes: llvm#109294 Fixes: SWDEV-485841 Change-Id: I328fb2edfca8110ea36c94812e60bc1d7663c266

This reverts commit 8f7e9f3. Change-Id: I5864c0da99feecc5658f997e75b13c77aa86cff8

This reverts commit f2f1fa7. Change-Id: Ibc1a99d7b73c376b09066809a3db6f80e3681ba6

…gue (llvm#109439) PRs llvm#69924 and llvm#72140 modified SIInstrInfo::isBasicBlockPrologue to skip over EXEC modifications and spills when allocating VGPRs. But treating VGPR spills as part of the prologue can confuse the register allocator as in llvm#109294, so restrict it to SGPR spills, which were inserted during SGPR allocation which is done in an earlier pass. Fixes: llvm#109294 Fixes: SWDEV-485841 Change-Id: Ice1ab75074aa380c13e07c452a2854f78ff37ce7

cdevadas requested review from arsenm, rampitec and qcolombet November 13, 2023 17:35

llvmbot added backend:AMDGPU llvm:regalloc labels Nov 13, 2023

rampitec reviewed Nov 13, 2023

View reviewed changes

cdevadas added a commit that referenced this pull request Nov 14, 2023

[AMDGPU] Precommit lit test for #72140.

8f7e9f3

cdevadas force-pushed the scalar-insertions-at-bb-top branch from c9cae57 to 8efa509 Compare November 14, 2023 21:22

rampitec reviewed Nov 14, 2023

View reviewed changes

Code refactoring and test fixup.

f7945c5

rampitec reviewed Nov 15, 2023

View reviewed changes

Improved a comment.

8f415b0

cdevadas force-pushed the scalar-insertions-at-bb-top branch from 3bc4387 to 8f415b0 Compare November 15, 2023 20:13

rampitec approved these changes Nov 15, 2023

View reviewed changes

cdevadas merged commit ce7fd49 into llvm:main Nov 16, 2023
2 checks passed

cdevadas mentioned this pull request Nov 16, 2023

[AMDGPU] Improve isBasicBlockPrologue helper function #69924

Merged

zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023

[AMDGPU] Precommit lit test for llvm#72140.

1fbe285

rocm-ci pushed a commit to ROCm/llvm-project that referenced this pull request Dec 15, 2023

[AMDGPU] Precommit lit test for llvm#72140.

12fa283

Change-Id: I8f5e2bc14ab9ae77d0c3a4571549eaff02bd9cf9

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Jul 29, 2024

Revert "[AMDGPU] RA inserted scalar instructions can be at the BB top (…

675ba5e

…llvm#72140)" This reverts commit ce7fd49. Change-Id: Iafb1f24fad78c477d3168a69cd8f9ccde6bc5038

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Jul 29, 2024

Revert "[AMDGPU] Precommit lit test for llvm#72140."

b633543

This reverts commit 8f7e9f3. Change-Id: Ibfb2589ae430d99ec16e482bad5f585f5af100b5

jrbyrnes pushed a commit to jrbyrnes/llvm-project that referenced this pull request Aug 16, 2024

Revert "[AMDGPU] RA inserted scalar instructions can be at the BB top (…

99e7093

…llvm#72140)" This reverts commit ce7fd49. Change-Id: Iafb1f24fad78c477d3168a69cd8f9ccde6bc5038

jrbyrnes pushed a commit to jrbyrnes/llvm-project that referenced this pull request Aug 16, 2024

Revert "[AMDGPU] Precommit lit test for llvm#72140."

164a5a0

This reverts commit 8f7e9f3. Change-Id: Ibfb2589ae430d99ec16e482bad5f585f5af100b5

jayfoad mentioned this pull request Sep 20, 2024

[AMDGPU] When allocating VGPRs, VGPR spills are not part of the prologue #109439

Merged

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Oct 23, 2024

Revert "Revert "[AMDGPU] Precommit lit test for llvm#72140.""

208cfa0

This reverts commit b633543. Change-Id: I4a7a15860efda59187e1daefd31eca806909c762

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Oct 23, 2024

Revert "Revert "[AMDGPU] RA inserted scalar instructions can be at th…

42cfb9f

…e BB top (llvm#72140)"" This reverts commit 675ba5e. Change-Id: I19b8d08af0dbea3d08d3ff310f90cad11fb80351

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Nov 22, 2024

Revert "[AMDGPU] Precommit lit test for llvm#72140."

f2f1fa7

This reverts commit 8f7e9f3. Change-Id: I5864c0da99feecc5658f997e75b13c77aa86cff8

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Nov 22, 2024

Revert "Revert "[AMDGPU] Precommit lit test for llvm#72140.""

355af9f

This reverts commit f2f1fa7. Change-Id: Ibc1a99d7b73c376b09066809a3db6f80e3681ba6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AMDGPU] RA inserted scalar instructions can be at the BB top #72140

[AMDGPU] RA inserted scalar instructions can be at the BB top #72140

cdevadas commented Nov 13, 2023

llvmbot commented Nov 13, 2023

llvmbot commented Nov 13, 2023

rampitec left a comment

rampitec Nov 13, 2023

qcolombet Nov 14, 2023

rampitec Nov 13, 2023

rampitec Nov 13, 2023

rampitec Nov 14, 2023

cdevadas commented Nov 14, 2023

rampitec Nov 14, 2023

arsenm Nov 14, 2023

arsenm Nov 15, 2023

rampitec Nov 14, 2023

cdevadas commented Nov 15, 2023

rampitec left a comment

rampitec Nov 15, 2023

rampitec left a comment

[AMDGPU] RA inserted scalar instructions can be at the BB top #72140

[AMDGPU] RA inserted scalar instructions can be at the BB top #72140

Conversation

cdevadas commented Nov 13, 2023

llvmbot commented Nov 13, 2023

llvmbot commented Nov 13, 2023

rampitec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdevadas commented Nov 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdevadas commented Nov 15, 2023

rampitec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rampitec left a comment

Choose a reason for hiding this comment