Skip to content

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 3, 2025

These instructions should now have proper representation
with separate instructions for operands which must be paired.

@llvmbot
Copy link
Member

llvmbot commented Sep 3, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

These instructions should now have proper representation
with separate instructions for operands which must be paired.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+9-13)
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 2b187c641da1c..981d31bc9b572 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -5960,7 +5960,7 @@ adjustAllocatableRegClass(const GCNSubtarget &ST, const SIRegisterInfo &RI,
   if ((IsAllocatable || !ST.hasGFX90AInsts()) &&
       (((TID.mayLoad() || TID.mayStore()) &&
         !(TID.TSFlags & SIInstrFlags::Spill)) ||
-       (TID.TSFlags & (SIInstrFlags::DS | SIInstrFlags::MIMG)))) {
+       (TID.TSFlags & SIInstrFlags::MIMG))) {
     switch (RCID) {
     case AMDGPU::AV_32RegClassID:
       RCID = AMDGPU::VGPR_32RegClassID;
@@ -5996,24 +5996,20 @@ const TargetRegisterClass *SIInstrInfo::getRegClass(const MCInstrDesc &TID,
     return nullptr;
   auto RegClass = TID.operands()[OpNum].RegClass;
   bool IsAllocatable = false;
-  if (TID.TSFlags & (SIInstrFlags::DS | SIInstrFlags::FLAT)) {
+  if (TID.TSFlags & SIInstrFlags::FLAT) {
     // vdst and vdata should be both VGPR or AGPR, same for the DS instructions
     // with two data operands. Request register class constrained to VGPR only
     // of both operands present as Machine Copy Propagation can not check this
     // constraint and possibly other passes too.
     //
-    // The check is limited to FLAT and DS because atomics in non-flat encoding
-    // have their vdst and vdata tied to be the same register.
-    const int VDstIdx = AMDGPU::getNamedOperandIdx(TID.Opcode,
-                                                   AMDGPU::OpName::vdst);
-    const int DataIdx = AMDGPU::getNamedOperandIdx(TID.Opcode,
-        (TID.TSFlags & SIInstrFlags::DS) ? AMDGPU::OpName::data0
-                                         : AMDGPU::OpName::vdata);
-    if (DataIdx != -1) {
-      IsAllocatable = VDstIdx != -1 || AMDGPU::hasNamedOperand(
-                                           TID.Opcode, AMDGPU::OpName::data1);
-    }
+    // The check is limited to FLAT because atomics in non-flat encoding have
+    // their vdst and vdata tied to be the same register, and DS instructions
+    // have separate instruction definitions with AGPR and VGPR operand lists.
+    IsAllocatable =
+        AMDGPU::hasNamedOperand(TID.Opcode, AMDGPU::OpName::vdata) &&
+        AMDGPU::hasNamedOperand(TID.Opcode, AMDGPU::OpName::vdst);
   }
+
   return adjustAllocatableRegClass(ST, RI, TID, RegClass, IsAllocatable);
 }
 

@arsenm arsenm force-pushed the users/arsenm/amdgpu/remove-ds-special-case-getRegClass branch from b9f01b2 to 1b6877b Compare September 4, 2025 03:25
@arsenm arsenm force-pushed the users/arsenm/amdgpu/add-agpr-ds-permute-insts branch from 75588de to 25ff1b6 Compare September 4, 2025 03:25
These are 2-data operations that need to use all-AGPR or all-VGPR
inputs. Stop defining them with AVLdSt data operands, and add _agpr
variants.
Correctly model these without AV_* operands. This is another
step towards removing the special casing in
TargetInstrInfo::getRegClass. Also add some tests for this.
These instructions should now have proper representation
with separate instructions for operands which must be paired.
@arsenm arsenm force-pushed the users/arsenm/amdgpu/remove-ds-special-case-getRegClass branch from 1b6877b to c7238cf Compare September 4, 2025 05:15
@arsenm arsenm force-pushed the users/arsenm/amdgpu/add-agpr-ds-permute-insts branch from 25ff1b6 to 22f1911 Compare September 4, 2025 05:15
Base automatically changed from users/arsenm/amdgpu/add-agpr-ds-permute-insts to main September 4, 2025 06:14
@arsenm arsenm merged commit a23a5b0 into main Sep 4, 2025
13 of 15 checks passed
@arsenm arsenm deleted the users/arsenm/amdgpu/remove-ds-special-case-getRegClass branch September 4, 2025 06:14
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.

3 participants