Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AMDGPU] Consolidate SGPRSpill and VGPRSpill into single Spill bit #81901

Merged
merged 6 commits into from
Feb 16, 2024

Conversation

CRobeck
Copy link
Member

@CRobeck CRobeck commented Feb 15, 2024

Follow on to #81525 in the series of consolidating bits in TSFlags.

Merge SGPRSpill and VGPRSpill into single Spill bit

Modify isSGPRSpill and isVGPRSpill helper functions to differentiate VGPR and SGPR spills:

Spill+SALU=SGPR Spill
Spill+VALU=VGPR Spill

The only exception here is SGPR spills to VGPRs which require an explicit instruction check.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 15, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Corbin Robeck (CRobeck)

Changes

Follow on to #81525 in the series of consolidating bits in TSFlags


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

5 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIDefines.h (+3-3)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrFormats.td (+6-5)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.h (+22-6)
  • (modified) llvm/lib/Target/AMDGPU/SIInstructions.td (+5-5)
diff --git a/llvm/lib/Target/AMDGPU/SIDefines.h b/llvm/lib/Target/AMDGPU/SIDefines.h
index ca6728cf3ddc67..2373726690cbf8 100644
--- a/llvm/lib/Target/AMDGPU/SIDefines.h
+++ b/llvm/lib/Target/AMDGPU/SIDefines.h
@@ -87,9 +87,9 @@ enum : uint64_t {
   FLAT = 1 << 24,
   DS = 1 << 25,
 
-  // Pseudo instruction formats.
-  VGPRSpill = 1 << 26,
-  SGPRSpill = 1 << 27,
+  // Combined SGPR/VGPR Spill bit
+  // logic to seperate them out is done in isSGPRSpill and isVGPRSpill
+  Spill = 1 << 26,
 
   // LDSDIR instruction format.
   LDSDIR = 1 << 28,
diff --git a/llvm/lib/Target/AMDGPU/SIInstrFormats.td b/llvm/lib/Target/AMDGPU/SIInstrFormats.td
index bdefcae278efa9..327eb89efcb88c 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrFormats.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrFormats.td
@@ -46,9 +46,8 @@ class InstSI <dag outs, dag ins, string asm = "",
   field bit FLAT = 0;
   field bit DS = 0;
 
-  // Pseudo instruction formats.
-  field bit VGPRSpill = 0;
-  field bit SGPRSpill = 0;
+  // Combined SGPR/VGPR spill bit
+  field bit Spill = 0;
 
   // LDSDIR instruction format.
   field bit LDSDIR = 0;
@@ -187,8 +186,10 @@ class InstSI <dag outs, dag ins, string asm = "",
   let TSFlags{24} = FLAT;
   let TSFlags{25} = DS;
 
-  let TSFlags{26} = VGPRSpill;
-  let TSFlags{27} = SGPRSpill;
+  let TSFlags{26} = Spill;
+
+  // Reserved, must be 0
+  let TSFlags{27} = 0;
 
   let TSFlags{28} = LDSDIR;
   let TSFlags{29} = VINTERP;
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index f5ec831234f2f9..62f42ceeda66a5 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -5424,7 +5424,7 @@ adjustAllocatableRegClass(const GCNSubtarget &ST, const SIRegisterInfo &RI,
                           bool IsAllocatable) {
   if ((IsAllocatable || !ST.hasGFX90AInsts() || !MRI.reservedRegsFrozen()) &&
       (((TID.mayLoad() || TID.mayStore()) &&
-        !(TID.TSFlags & SIInstrFlags::VGPRSpill)) ||
+        !(TID.TSFlags & SIInstrFlags::Spill)) ||
        (TID.TSFlags & (SIInstrFlags::DS | SIInstrFlags::MIMG)))) {
     switch (RCID) {
     case AMDGPU::AV_32RegClassID:
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index 7a6c28421c8d7a..b639aaf733972c 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -708,25 +708,41 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
     return get(Opcode).TSFlags & SIInstrFlags::DisableWQM;
   }
 
+  // SI_SPILL_S32_TO_VGPR and SI_RESTORE_S32_FROM_VGPR form a special case of
+  // SGPRs spilling to VGPRs which are SGPR spills but from VALU instructions
+  // therefore we need an explicit check for them since just checking if the
+  // Spill bit is set and what instruction type it came from miss classifies
+  // them.
   static bool isVGPRSpill(const MachineInstr &MI) {
-    return MI.getDesc().TSFlags & SIInstrFlags::VGPRSpill;
+    return MI.getOpcode() != AMDGPU::SI_SPILL_S32_TO_VGPR &&
+           MI.getOpcode() != AMDGPU::SI_RESTORE_S32_FROM_VGPR &&
+           ((MI.getDesc().TSFlags & SIInstrFlags::Spill) &&
+            (MI.getDesc().TSFlags & SIInstrFlags::VALU));
   }
 
   bool isVGPRSpill(uint16_t Opcode) const {
-    return get(Opcode).TSFlags & SIInstrFlags::VGPRSpill;
+    return Opcode != AMDGPU::SI_SPILL_S32_TO_VGPR &&
+           Opcode != AMDGPU::SI_RESTORE_S32_FROM_VGPR &&
+           ((get(Opcode).TSFlags & SIInstrFlags::Spill) &&
+            (get(Opcode).TSFlags & SIInstrFlags::VALU));
   }
 
   static bool isSGPRSpill(const MachineInstr &MI) {
-    return MI.getDesc().TSFlags & SIInstrFlags::SGPRSpill;
+    return MI.getOpcode() == AMDGPU::SI_SPILL_S32_TO_VGPR ||
+           MI.getOpcode() == AMDGPU::SI_RESTORE_S32_FROM_VGPR ||
+           ((MI.getDesc().TSFlags & SIInstrFlags::Spill) &&
+            (MI.getDesc().TSFlags & SIInstrFlags::SALU));
   }
 
   bool isSGPRSpill(uint16_t Opcode) const {
-    return get(Opcode).TSFlags & SIInstrFlags::SGPRSpill;
+    return Opcode == AMDGPU::SI_SPILL_S32_TO_VGPR ||
+           Opcode == AMDGPU::SI_RESTORE_S32_FROM_VGPR ||
+           ((get(Opcode).TSFlags & SIInstrFlags::Spill) &&
+            (get(Opcode).TSFlags & SIInstrFlags::SALU));
   }
 
   bool isSpillOpcode(uint16_t Opcode) const {
-    return get(Opcode).TSFlags &
-           (SIInstrFlags::SGPRSpill | SIInstrFlags::VGPRSpill);
+    return get(Opcode).TSFlags & SIInstrFlags::Spill;
   }
 
   static bool isWWMRegSpillOpcode(uint16_t Opcode) {
diff --git a/llvm/lib/Target/AMDGPU/SIInstructions.td b/llvm/lib/Target/AMDGPU/SIInstructions.td
index b593b7dbfe0827..565af36bc523e4 100644
--- a/llvm/lib/Target/AMDGPU/SIInstructions.td
+++ b/llvm/lib/Target/AMDGPU/SIInstructions.td
@@ -895,7 +895,7 @@ def V_INDIRECT_REG_READ_GPR_IDX_B32_V16 : V_INDIRECT_REG_READ_GPR_IDX_pseudo<VRe
 def V_INDIRECT_REG_READ_GPR_IDX_B32_V32 : V_INDIRECT_REG_READ_GPR_IDX_pseudo<VReg_1024>;
 
 multiclass SI_SPILL_SGPR <RegisterClass sgpr_class> {
-  let UseNamedOperandTable = 1, SGPRSpill = 1, Uses = [EXEC] in {
+  let UseNamedOperandTable = 1, Spill = 1, SALU = 1, Uses = [EXEC] in {
     def _SAVE : PseudoInstSI <
       (outs),
       (ins sgpr_class:$data, i32imm:$addr)> {
@@ -931,7 +931,7 @@ defm SI_SPILL_S384 : SI_SPILL_SGPR <SReg_384>;
 defm SI_SPILL_S512 : SI_SPILL_SGPR <SReg_512>;
 defm SI_SPILL_S1024 : SI_SPILL_SGPR <SReg_1024>;
 
-let SGPRSpill = 1, VALU = 1, isConvergent = 1 in {
+let Spill = 1, VALU = 1, isConvergent = 1 in {
 def SI_SPILL_S32_TO_VGPR : PseudoInstSI <(outs VGPR_32:$vdst),
   (ins SReg_32:$src0, i32imm:$src1, VGPR_32:$vdst_in)> {
   let Size = 4;
@@ -951,13 +951,13 @@ def SI_RESTORE_S32_FROM_VGPR : PseudoInstSI <(outs SReg_32:$sdst),
   let mayLoad = 0;
   let mayStore = 0;
 }
-} // End SGPRSpill = 1, VALU = 1, isConvergent = 1
+} // End Spill = 1, VALU = 1, isConvergent = 1
 
 // VGPR or AGPR spill instructions. In case of AGPR spilling a temp register
 // needs to be used and an extra instruction to move between VGPR and AGPR.
 // UsesTmp adds to the total size of an expanded spill in this case.
 multiclass SI_SPILL_VGPR <RegisterClass vgpr_class, bit UsesTmp = 0> {
-  let UseNamedOperandTable = 1, VGPRSpill = 1,
+  let UseNamedOperandTable = 1, Spill = 1, VALU = 1,
        SchedRW = [WriteVMEM] in {
     def _SAVE : VPseudoInstSI <
       (outs),
@@ -983,7 +983,7 @@ multiclass SI_SPILL_VGPR <RegisterClass vgpr_class, bit UsesTmp = 0> {
       // Size field is unsigned char and cannot fit more.
       let Size = !if(!le(MaxSize, 256), MaxSize, 252);
     }
-  } // End UseNamedOperandTable = 1, VGPRSpill = 1, SchedRW = [WriteVMEM]
+  } // End UseNamedOperandTable = 1, Spill = 1, VALU = 1, SchedRW = [WriteVMEM]
 }
 
 defm SI_SPILL_V32  : SI_SPILL_VGPR <VGPR_32>;

Copy link

github-actions bot commented Feb 16, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@CRobeck CRobeck force-pushed the users/crobeck/consolidate-vgpr-sgpr-bit branch from c7c0303 to 212570f Compare February 16, 2024 13:50
@CRobeck CRobeck merged commit 2d9f350 into main Feb 16, 2024
4 checks passed
@CRobeck CRobeck deleted the users/crobeck/consolidate-vgpr-sgpr-bit branch February 16, 2024 18:33
return MI.getDesc().TSFlags & SIInstrFlags::VGPRSpill;
return MI.getOpcode() != AMDGPU::SI_SPILL_S32_TO_VGPR &&
MI.getOpcode() != AMDGPU::SI_RESTORE_S32_FROM_VGPR &&
(isSpill(MI) && isVALU(MI));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check these first?

return get(Opcode).TSFlags & SIInstrFlags::VGPRSpill;
return Opcode != AMDGPU::SI_SPILL_S32_TO_VGPR &&
Opcode != AMDGPU::SI_RESTORE_S32_FROM_VGPR &&
(isSpill(Opcode) && isVALU(Opcode));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check these first?

return MI.getDesc().TSFlags & SIInstrFlags::SGPRSpill;
return MI.getOpcode() == AMDGPU::SI_SPILL_S32_TO_VGPR ||
MI.getOpcode() == AMDGPU::SI_RESTORE_S32_FROM_VGPR ||
(isSpill(MI) && isSALU(MI));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check these first?

return get(Opcode).TSFlags & SIInstrFlags::SGPRSpill;
return Opcode == AMDGPU::SI_SPILL_S32_TO_VGPR ||
Opcode == AMDGPU::SI_RESTORE_S32_FROM_VGPR ||
(isSpill(Opcode) && isSALU(Opcode));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check these first?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants