Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 3, 2025

Start stripping out the uses of getLdStRegisterOperand. This
added a confusing level of indirection where the class at the
definition point was not the actual class used. This was also
pulling in the AV class usage for targets where it isn't
relevant. This was also inflexible for special cases.

Also fixes using default arguments which only served to wrap the
class argument in a RegisterOperand.

This should be done for all the memory instructions.

Copy link
Contributor Author

arsenm commented Sep 3, 2025

@llvmbot
Copy link
Member

llvmbot commented Sep 3, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

Start stripping out the uses of getLdStRegisterOperand. This
added a confusing level of indirection where the class at the
definition point was not the actual class used. This was also
pulling in the AV class usage for targets where it isn't
relevant. This was also inflexible for special cases.

Also fixes using default arguments which only served to wrap the
class argument in a RegisterOperand.

This should be done for all the memory instructions.


Patch is 33.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/156580.diff

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/DSInstructions.td (+166-153)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.td (+11)
  • (modified) llvm/lib/Target/AMDGPU/SIRegisterInfo.td (+20)
diff --git a/llvm/lib/Target/AMDGPU/DSInstructions.td b/llvm/lib/Target/AMDGPU/DSInstructions.td
index 7552326c39468..960f3282fb6f6 100644
--- a/llvm/lib/Target/AMDGPU/DSInstructions.td
+++ b/llvm/lib/Target/AMDGPU/DSInstructions.td
@@ -130,10 +130,10 @@ class DS_Real <DS_Pseudo ps, string opName = ps.Mnemonic> :
 
 // DS Pseudo instructions
 
-class DS_0A1D_NORET<string opName, RegisterClass rc = VGPR_32>
+class DS_0A1D_NORET<string opName, RegisterOperand rc = AVLdSt_32>
 : DS_Pseudo<opName,
   (outs),
-  (ins getLdStRegisterOperand<rc>.ret:$data0, Offset:$offset, gds:$gds),
+  (ins rc:$data0, Offset:$offset, gds:$gds),
   " $data0$offset$gds"> {
 
   let has_addr = 0;
@@ -141,10 +141,10 @@ class DS_0A1D_NORET<string opName, RegisterClass rc = VGPR_32>
   let has_vdst = 0;
 }
 
-class DS_1A1D_NORET<string opName, RegisterClass rc = VGPR_32>
+class DS_1A1D_NORET<string opName, RegisterOperand rc = AVLdSt_32>
 : DS_Pseudo<opName,
   (outs),
-  (ins VGPR_32:$addr, getLdStRegisterOperand<rc>.ret:$data0, Offset:$offset, gds:$gds),
+  (ins VGPR_32:$addr, rc:$data0, Offset:$offset, gds:$gds),
   " $addr, $data0$offset$gds"> {
 
   let has_data1 = 0;
@@ -152,7 +152,7 @@ class DS_1A1D_NORET<string opName, RegisterClass rc = VGPR_32>
   let IsAtomicNoRet = 1;
 }
 
-multiclass DS_1A1D_NORET_mc<string opName, RegisterClass rc = VGPR_32> {
+multiclass DS_1A1D_NORET_mc<string opName, RegisterOperand rc = AVLdSt_32> {
   def "" : DS_1A1D_NORET<opName, rc>;
 
   let has_m0_read = 0 in {
@@ -160,23 +160,23 @@ multiclass DS_1A1D_NORET_mc<string opName, RegisterClass rc = VGPR_32> {
   }
 }
 
-multiclass DS_1A1D_NORET_t16<string opName, RegisterClass rc = VGPR_32>
+multiclass DS_1A1D_NORET_t16<string opName, RegisterOperand rc = AVLdSt_32>
 : DS_1A1D_NORET_mc<opName, rc> {
   let has_m0_read = 0 in {
     let True16Predicate = UseRealTrue16Insts in {
-      def "_t16" : DS_1A1D_NORET<opName#"_t16", VGPR_16>,
+      def "_t16" : DS_1A1D_NORET<opName#"_t16", VGPROp_16>,
         True16D16Table<NAME#"_D16_HI", NAME#"_gfx9">;
     }
   }
 }
 
-multiclass DS_1A1D_NORET_mc_gfx9<string opName, RegisterClass rc = VGPR_32> {
+multiclass DS_1A1D_NORET_mc_gfx9<string opName, RegisterOperand rc = AVLdSt_32> {
   let has_m0_read = 0 in {
     def "" : DS_1A1D_NORET<opName, rc>;
   }
 }
 
-class DS_1A2D_NORET<string opName, RegisterClass data_op = VGPR_32>
+class DS_1A2D_NORET<string opName, RegisterOperand data_op = VGPROp_32>
 : DS_Pseudo<opName,
   (outs),
   (ins VGPR_32:$addr, data_op:$data0, data_op:$data1, Offset:$offset, gds:$gds),
@@ -186,7 +186,11 @@ class DS_1A2D_NORET<string opName, RegisterClass data_op = VGPR_32>
   let IsAtomicNoRet = 1;
 }
 
-multiclass DS_1A2D_NORET_mc<string opName, RegisterClass rc = VGPR_32> {
+// DS_xx2D cases should only be instantiated with VGPR operand classes.
+multiclass DS_1A2D_NORET_mc<string opName, RegisterOperand rc = VGPROp_32> {
+  assert OperandIsVGPR<rc>.ret,
+         "DS with 2 data operands should be declared with VGPRs";
+
   def "" : DS_1A2D_NORET<opName, rc>;
 
   let has_m0_read = 0 in {
@@ -194,12 +198,12 @@ multiclass DS_1A2D_NORET_mc<string opName, RegisterClass rc = VGPR_32> {
 
     // All data operands are replaced with AGPRs in this form.
     let SubtargetPredicate = isGFX90APlus in {
-      def _agpr : DS_1A2D_NORET<opName, getEquivalentAGPRClass<rc>.ret>;
+      def _agpr : DS_1A2D_NORET<opName, getEquivalentAGPROperand<rc>.ret>;
     }
   }
 }
 
-class DS_1A2D_Off8_NORET <string opName, RegisterClass data_op = VGPR_32>
+class DS_1A2D_Off8_NORET <string opName, RegisterOperand data_op = VGPROp_32>
 : DS_Pseudo<opName,
   (outs),
   (ins VGPR_32:$addr, data_op:$data0, data_op:$data1,
@@ -210,21 +214,23 @@ class DS_1A2D_Off8_NORET <string opName, RegisterClass data_op = VGPR_32>
   let has_offset = 0;
 }
 
-multiclass DS_1A2D_Off8_NORET_mc <string opName, RegisterClass rc = VGPR_32> {
+multiclass DS_1A2D_Off8_NORET_mc <string opName, RegisterOperand rc = VGPROp_32> {
+  assert OperandIsVGPR<rc>.ret,
+         "DS with 2 data operands should be declared with VGPRs";
+
   def "" : DS_1A2D_Off8_NORET<opName, rc>;
 
   let has_m0_read = 0 in {
     def _gfx9 : DS_1A2D_Off8_NORET<opName, rc>;
 
     let SubtargetPredicate = isGFX90APlus in {
-      def _agpr : DS_1A2D_Off8_NORET<opName, getEquivalentAGPRClass<rc>.ret>;
+      def _agpr : DS_1A2D_Off8_NORET<opName, getEquivalentAGPROperand<rc>.ret>;
     }
   }
 }
 
-class DS_0A1D_RET_GDS<string opName, RegisterClass rc = VGPR_32, RegisterClass src = rc,
-                  RegisterOperand dst_op = getLdStRegisterOperand<rc>.ret,
-                  RegisterOperand src_op = getLdStRegisterOperand<src>.ret>
+class DS_0A1D_RET_GDS<string opName, RegisterOperand dst_op = AVLdSt_32,
+                                     RegisterOperand src_op = dst_op>
 : DS_Pseudo<opName,
   (outs dst_op:$vdst),
   (ins src_op:$data0, Offset:$offset),
@@ -237,8 +243,7 @@ class DS_0A1D_RET_GDS<string opName, RegisterClass rc = VGPR_32, RegisterClass s
   let hasSideEffects = 1;
 }
 
-class DS_1A1D_RET <string opName, RegisterClass rc = VGPR_32,
-                  RegisterOperand data_op = getLdStRegisterOperand<rc>.ret>
+class DS_1A1D_RET <string opName, RegisterOperand data_op = AVLdSt_32>
 : DS_Pseudo<opName,
   (outs data_op:$vdst),
   (ins VGPR_32:$addr, data_op:$data0, Offset:$offset, gds:$gds),
@@ -248,7 +253,7 @@ class DS_1A1D_RET <string opName, RegisterClass rc = VGPR_32,
   let IsAtomicRet = 1;
 }
 
-multiclass DS_1A1D_RET_mc <string opName, RegisterClass rc = VGPR_32> {
+multiclass DS_1A1D_RET_mc <string opName, RegisterOperand rc = AVLdSt_32> {
   def "" : DS_1A1D_RET<opName, rc>;
 
   let has_m0_read = 0 in {
@@ -256,15 +261,15 @@ multiclass DS_1A1D_RET_mc <string opName, RegisterClass rc = VGPR_32> {
   }
 }
 
-multiclass DS_1A1D_RET_mc_gfx9 <string opName, RegisterClass rc = VGPR_32> {
+multiclass DS_1A1D_RET_mc_gfx9 <string opName, RegisterOperand rc = AVLdSt_32> {
   let has_m0_read = 0 in {
     def "" : DS_1A1D_RET<opName, rc>;
   }
 }
 
 class DS_1A2D_RET<string opName,
-                  RegisterClass dst_rc = VGPR_32,
-                  RegisterClass src_rc = dst_rc>: DS_Pseudo<opName,
+                  RegisterOperand dst_rc = VGPROp_32,
+                  RegisterOperand src_rc = dst_rc>: DS_Pseudo<opName,
   (outs dst_rc:$vdst),
   (ins VGPR_32:$addr, src_rc:$data0, src_rc:$data1, Offset:$offset, gds:$gds),
   " $vdst, $addr, $data0, $data1$offset$gds"> {
@@ -273,20 +278,23 @@ class DS_1A2D_RET<string opName,
 }
 
 multiclass DS_1A2D_RET_mc<string opName,
-                          RegisterClass dst_rc = VGPR_32,
-                          RegisterClass src_rc = dst_rc> {
+                          RegisterOperand dst_rc = VGPROp_32,
+                          RegisterOperand src_rc = dst_rc> {
+  assert !and(OperandIsVGPR<dst_rc>.ret, OperandIsVGPR<src_rc>.ret),
+         "DS with 2 data operands should be declared with VGPRs";
+
   def "" : DS_1A2D_RET<opName, dst_rc, src_rc>;
 
   let has_m0_read = 0 in {
     def _gfx9 : DS_1A2D_RET<opName, dst_rc, src_rc>;
-    def _agpr : DS_1A2D_RET<opName, getEquivalentAGPRClass<dst_rc>.ret,
-                                    getEquivalentAGPRClass<src_rc>.ret>;
+    def _agpr : DS_1A2D_RET<opName, getEquivalentAGPROperand<dst_rc>.ret,
+                                    getEquivalentAGPROperand<src_rc>.ret>;
   }
 }
 
 class DS_1A2D_Off8_RET<string opName,
-                       RegisterClass dst_rc = VGPR_32,
-                       RegisterClass src_rc = dst_rc>
+                       RegisterOperand dst_rc = VGPROp_32,
+                       RegisterOperand src_rc = dst_rc>
 : DS_Pseudo<opName,
   (outs dst_rc:$vdst),
   (ins VGPR_32:$addr, src_rc:$data0, src_rc:$data1, Offset0:$offset0, Offset1:$offset1, gds:$gds),
@@ -296,24 +304,26 @@ class DS_1A2D_Off8_RET<string opName,
 }
 
 multiclass DS_1A2D_Off8_RET_mc<string opName,
-                               RegisterClass dst_rc = VGPR_32,
-                               RegisterClass src_rc = dst_rc> {
+                               RegisterOperand dst_rc = VGPROp_32,
+                               RegisterOperand src_rc = dst_rc> {
+  assert !and(OperandIsVGPR<dst_rc>.ret, OperandIsVGPR<src_rc>.ret)  ,
+         "DS with 2 data operands should be declared with VGPRs";
+
   def "" : DS_1A2D_Off8_RET<opName, dst_rc, src_rc>;
 
   let has_m0_read = 0 in {
     def _gfx9 : DS_1A2D_Off8_RET<opName, dst_rc, src_rc>;
-    def _agpr : DS_1A2D_Off8_RET<opName, getEquivalentAGPRClass<dst_rc>.ret,
-                                         getEquivalentAGPRClass<src_rc>.ret>;
+    def _agpr : DS_1A2D_Off8_RET<opName, getEquivalentAGPROperand<dst_rc>.ret,
+                                         getEquivalentAGPROperand<src_rc>.ret>;
   }
 }
 
 class DS_BVH_STACK<string opName,
-                   RegisterClass vdst_rc,
-                   RegisterClass data1_rc>
+                   RegisterOperand vdst_rc,
+                   RegisterOperand data1_rc>
 : DS_Pseudo<opName,
-  (outs getLdStRegisterOperand<vdst_rc>.ret:$vdst, VGPR_32:$addr),
-  (ins VGPR_32:$addr_in, getLdStRegisterOperand<VGPR_32>.ret:$data0,
-   data1_rc:$data1, Offset:$offset),
+  (outs vdst_rc:$vdst, VGPR_32:$addr),
+  (ins VGPR_32:$addr_in, VGPR_32:$data0, data1_rc:$data1, Offset:$offset),
   " $vdst, $addr, $data0, $data1$offset"> {
   let Constraints = "$addr = $addr_in";
   let has_gds = 0;
@@ -323,8 +333,8 @@ class DS_BVH_STACK<string opName,
   let SchedRW = [WriteLDS, WriteLDS];
 }
 
-class DS_1A_RET<string opName, RegisterClass rc = VGPR_32, bit HasTiedOutput = 0, Operand ofs = Offset,
-                RegisterOperand data_op = getLdStRegisterOperand<rc>.ret>
+class DS_1A_RET<string opName, RegisterOperand data_op = AVLdSt_32,
+                bit HasTiedOutput = 0, Operand ofs = Offset>
 : DS_Pseudo<opName,
   (outs data_op:$vdst),
   !if(HasTiedOutput,
@@ -336,7 +346,8 @@ class DS_1A_RET<string opName, RegisterClass rc = VGPR_32, bit HasTiedOutput = 0
   let has_data1 = 0;
 }
 
-multiclass DS_1A_RET_mc<string opName, RegisterClass rc = VGPR_32, bit HasTiedOutput = 0, Operand ofs = Offset> {
+multiclass DS_1A_RET_mc<string opName, RegisterOperand rc = AVLdSt_32,
+                        bit HasTiedOutput = 0, Operand ofs = Offset> {
   def "" : DS_1A_RET<opName, rc, HasTiedOutput, ofs>;
 
   let has_m0_read = 0 in {
@@ -344,27 +355,28 @@ multiclass DS_1A_RET_mc<string opName, RegisterClass rc = VGPR_32, bit HasTiedOu
   }
 }
 
-multiclass DS_1A_RET_t16<string opName, RegisterClass rc = VGPR_32, bit HasTiedOutput = 0, Operand ofs = Offset>
+multiclass DS_1A_RET_t16<string opName, RegisterOperand rc = AVLdSt_32,
+                         bit HasTiedOutput = 0, Operand ofs = Offset>
 : DS_1A_RET_mc<opName, rc, HasTiedOutput, ofs> {
   let has_m0_read = 0 in {
     let True16Predicate = UseRealTrue16Insts in {
-      def "_t16" : DS_1A_RET<opName#"_t16", VGPR_16, HasTiedOutput, ofs>, True16D16Table<NAME#"_D16_HI", NAME#"_D16">;
+      def "_t16" : DS_1A_RET<opName#"_t16", VGPROp_16, HasTiedOutput, ofs>, True16D16Table<NAME#"_D16_HI", NAME#"_D16">;
     }
   }
 }
 
-multiclass DS_1A_RET_NoM0<string opName, RegisterClass rc = VGPR_32> {
+multiclass DS_1A_RET_NoM0<string opName, RegisterOperand rc = VGPROp_32> {
   let has_m0_read = 0 in {
     def "" : DS_1A_RET<opName, rc>;
   }
 }
 
-class DS_1A_RET_Tied<string opName, RegisterClass rc = VGPR_32> :
+class DS_1A_RET_Tied<string opName, RegisterOperand rc = AVLdSt_32> :
   DS_1A_RET<opName, rc, 1>;
 
-class DS_1A_Off8_RET <string opName, RegisterClass rc = VGPR_32>
+class DS_1A_Off8_RET <string opName, RegisterOperand rc = AVLdSt_32>
 : DS_Pseudo<opName,
-  (outs getLdStRegisterOperand<rc>.ret:$vdst),
+  (outs rc:$vdst),
   (ins VGPR_32:$addr, Offset0:$offset0, Offset1:$offset1, gds:$gds),
   " $vdst, $addr$offset0$offset1$gds"> {
 
@@ -373,7 +385,7 @@ class DS_1A_Off8_RET <string opName, RegisterClass rc = VGPR_32>
   let has_data1 = 0;
 }
 
-multiclass DS_1A_Off8_RET_mc <string opName, RegisterClass rc = VGPR_32> {
+multiclass DS_1A_Off8_RET_mc <string opName, RegisterOperand rc = VGPROp_32> {
   def "" : DS_1A_Off8_RET<opName, rc>;
 
   let has_m0_read = 0 in {
@@ -382,7 +394,7 @@ multiclass DS_1A_Off8_RET_mc <string opName, RegisterClass rc = VGPR_32> {
 }
 
 class DS_1A_RET_GDS <string opName> : DS_Pseudo<opName,
-  (outs getLdStRegisterOperand<VGPR_32>.ret:$vdst),
+  (outs AVLdSt_32:$vdst),
   (ins VGPR_32:$addr, Offset:$offset),
   " $vdst, $addr$offset gds"> {
 
@@ -407,7 +419,7 @@ class DS_1A_Off16_NORET <string opName>
 }
 
 class DS_0A_RET <string opName> : DS_Pseudo<opName,
-  (outs getLdStRegisterOperand<VGPR_32>.ret:$vdst),
+  (outs AVLdSt_32:$vdst),
   (ins Offset:$offset, gds:$gds),
   " $vdst$offset$gds"> {
 
@@ -462,7 +474,7 @@ class DS_GWS_0D <string opName>
 
 class DS_GWS_1D <string opName>
 : DS_GWS<opName,
-  (ins getLdStRegisterOperand<VGPR_32>.ret:$data0, Offset:$offset),
+  (ins AVLdSt_32:$data0, Offset:$offset),
   " $data0$offset gds"> {
 
   let has_gws_data0 = 1;
@@ -487,7 +499,7 @@ class DS_VOID <string opName> : DS_Pseudo<opName,
 }
 
 class DS_1A1D_PERMUTE <string opName, SDPatternOperator node = null_frag,
-                       RegisterOperand data_op = getLdStRegisterOperand<VGPR_32>.ret>
+                       RegisterOperand data_op = AVLdSt_32>
 : DS_Pseudo<opName,
   (outs data_op:$vdst),
   (ins VGPR_32:$addr, data_op:$data0, Offset:$offset),
@@ -604,100 +616,100 @@ def DS_WRITE_ADDTID_B32 : DS_0A1D_NORET<"ds_write_addtid_b32">;
 } // End mayLoad = 0
 
 let SubtargetPredicate = HasLdsAtomicAddF64 in {
-  defm DS_ADD_F64     : DS_1A1D_NORET_mc_gfx9<"ds_add_f64", VReg_64>;
-  defm DS_ADD_RTN_F64 : DS_1A1D_RET_mc_gfx9<"ds_add_rtn_f64", VReg_64>;
+  defm DS_ADD_F64     : DS_1A1D_NORET_mc_gfx9<"ds_add_f64", AVLdSt_64>;
+  defm DS_ADD_RTN_F64 : DS_1A1D_RET_mc_gfx9<"ds_add_rtn_f64", AVLdSt_64>;
 } // End SubtargetPredicate = HasLdsAtomicAddF64
 
 let SubtargetPredicate = HasAtomicDsPkAdd16Insts in {
   defm DS_PK_ADD_F16      : DS_1A1D_NORET_mc_gfx9<"ds_pk_add_f16">;
-  defm DS_PK_ADD_RTN_F16  : DS_1A1D_RET_mc_gfx9<"ds_pk_add_rtn_f16", VGPR_32>;
+  defm DS_PK_ADD_RTN_F16  : DS_1A1D_RET_mc_gfx9<"ds_pk_add_rtn_f16">;
   defm DS_PK_ADD_BF16     : DS_1A1D_NORET_mc_gfx9<"ds_pk_add_bf16">;
-  defm DS_PK_ADD_RTN_BF16 : DS_1A1D_RET_mc_gfx9<"ds_pk_add_rtn_bf16", VGPR_32>;
+  defm DS_PK_ADD_RTN_BF16 : DS_1A1D_RET_mc_gfx9<"ds_pk_add_rtn_bf16">;
 } // End SubtargetPredicate = HasAtomicDsPkAdd16Insts
 
 defm DS_CMPSTORE_B32     : DS_1A2D_NORET_mc<"ds_cmpstore_b32">;
 defm DS_CMPSTORE_F32     : DS_1A2D_NORET_mc<"ds_cmpstore_f32">;
-defm DS_CMPSTORE_B64     : DS_1A2D_NORET_mc<"ds_cmpstore_b64", VReg_64>;
-defm DS_CMPSTORE_F64     : DS_1A2D_NORET_mc<"ds_cmpstore_f64", VReg_64>;
-defm DS_CMPSTORE_RTN_B32 : DS_1A2D_RET_mc<"ds_cmpstore_rtn_b32", VGPR_32>;
-defm DS_CMPSTORE_RTN_F32 : DS_1A2D_RET_mc<"ds_cmpstore_rtn_f32", VGPR_32>;
-defm DS_CMPSTORE_RTN_B64  : DS_1A2D_RET_mc<"ds_cmpstore_rtn_b64", VReg_64>;
-defm DS_CMPSTORE_RTN_F64  : DS_1A2D_RET_mc<"ds_cmpstore_rtn_f64", VReg_64>;
+defm DS_CMPSTORE_B64     : DS_1A2D_NORET_mc<"ds_cmpstore_b64", VGPROp_64>;
+defm DS_CMPSTORE_F64     : DS_1A2D_NORET_mc<"ds_cmpstore_f64", VGPROp_64>;
+defm DS_CMPSTORE_RTN_B32 : DS_1A2D_RET_mc<"ds_cmpstore_rtn_b32">;
+defm DS_CMPSTORE_RTN_F32 : DS_1A2D_RET_mc<"ds_cmpstore_rtn_f32">;
+defm DS_CMPSTORE_RTN_B64  : DS_1A2D_RET_mc<"ds_cmpstore_rtn_b64", VGPROp_64>;
+defm DS_CMPSTORE_RTN_F64  : DS_1A2D_RET_mc<"ds_cmpstore_rtn_f64", VGPROp_64>;
 
 defm DS_MSKOR_B32     : DS_1A2D_NORET_mc<"ds_mskor_b32">;
 defm DS_CMPST_B32     : DS_1A2D_NORET_mc<"ds_cmpst_b32">;
 defm DS_CMPST_F32     : DS_1A2D_NORET_mc<"ds_cmpst_f32">;
 
-defm DS_ADD_U64       : DS_1A1D_NORET_mc<"ds_add_u64", VReg_64>;
-defm DS_SUB_U64       : DS_1A1D_NORET_mc<"ds_sub_u64", VReg_64>;
-defm DS_RSUB_U64      : DS_1A1D_NORET_mc<"ds_rsub_u64", VReg_64>;
-defm DS_INC_U64       : DS_1A1D_NORET_mc<"ds_inc_u64", VReg_64>;
-defm DS_DEC_U64       : DS_1A1D_NORET_mc<"ds_dec_u64", VReg_64>;
-defm DS_MIN_I64       : DS_1A1D_NORET_mc<"ds_min_i64", VReg_64>;
-defm DS_MAX_I64       : DS_1A1D_NORET_mc<"ds_max_i64", VReg_64>;
-defm DS_MIN_U64       : DS_1A1D_NORET_mc<"ds_min_u64", VReg_64>;
-defm DS_MAX_U64       : DS_1A1D_NORET_mc<"ds_max_u64", VReg_64>;
-defm DS_AND_B64       : DS_1A1D_NORET_mc<"ds_and_b64", VReg_64>;
-defm DS_OR_B64        : DS_1A1D_NORET_mc<"ds_or_b64", VReg_64>;
-defm DS_XOR_B64       : DS_1A1D_NORET_mc<"ds_xor_b64", VReg_64>;
-defm DS_MSKOR_B64     : DS_1A2D_NORET_mc<"ds_mskor_b64", VReg_64>;
+defm DS_ADD_U64       : DS_1A1D_NORET_mc<"ds_add_u64", AVLdSt_64>;
+defm DS_SUB_U64       : DS_1A1D_NORET_mc<"ds_sub_u64", AVLdSt_64>;
+defm DS_RSUB_U64      : DS_1A1D_NORET_mc<"ds_rsub_u64", AVLdSt_64>;
+defm DS_INC_U64       : DS_1A1D_NORET_mc<"ds_inc_u64", AVLdSt_64>;
+defm DS_DEC_U64       : DS_1A1D_NORET_mc<"ds_dec_u64", AVLdSt_64>;
+defm DS_MIN_I64       : DS_1A1D_NORET_mc<"ds_min_i64", AVLdSt_64>;
+defm DS_MAX_I64       : DS_1A1D_NORET_mc<"ds_max_i64", AVLdSt_64>;
+defm DS_MIN_U64       : DS_1A1D_NORET_mc<"ds_min_u64", AVLdSt_64>;
+defm DS_MAX_U64       : DS_1A1D_NORET_mc<"ds_max_u64", AVLdSt_64>;
+defm DS_AND_B64       : DS_1A1D_NORET_mc<"ds_and_b64", AVLdSt_64>;
+defm DS_OR_B64        : DS_1A1D_NORET_mc<"ds_or_b64", AVLdSt_64>;
+defm DS_XOR_B64       : DS_1A1D_NORET_mc<"ds_xor_b64", AVLdSt_64>;
+defm DS_MSKOR_B64     : DS_1A2D_NORET_mc<"ds_mskor_b64", VGPROp_64>;
 let mayLoad = 0 in {
-defm DS_WRITE_B64     : DS_1A1D_NORET_mc<"ds_write_b64", VReg_64>;
-defm DS_WRITE2_B64    : DS_1A2D_Off8_NORET_mc<"ds_write2_b64", VReg_64>;
-defm DS_WRITE2ST64_B64: DS_1A2D_Off8_NORET_mc<"ds_write2st64_b64", VReg_64>;
+defm DS_WRITE_B64     : DS_1A1D_NORET_mc<"ds_write_b64", AVLdSt_64>;
+defm DS_WRITE2_B64    : DS_1A2D_Off8_NORET_mc<"ds_write2_b64", VGPROp_64>;
+defm DS_WRITE2ST64_B64: DS_1A2D_Off8_NORET_mc<"ds_write2st64_b64", VGPROp_64>;
 }
-defm DS_CMPST_B64     : DS_1A2D_NORET_mc<"ds_cmpst_b64", VReg_64>;
-defm DS_CMPST_F64     : DS_1A2D_NORET_mc<"ds_cmpst_f64", VReg_64>;
-defm DS_MIN_F64       : DS_1A1D_NORET_mc<"ds_min_f64", VReg_64>;
-defm DS_MAX_F64       : DS_1A1D_NORET_mc<"ds_max_f64", VReg_64>;
+defm DS_CMPST_B64     : DS_1A2D_NORET_mc<"ds_cmpst_b64", VGPROp_64>;
+defm DS_CMPST_F64     : DS_1A2D_NORET_mc<"ds_cmpst_f64", VGPROp_64>;
+defm DS_MIN_F64       : DS_1A1D_NORET_mc<"ds_min_f64", AVLdSt_64>;
+defm DS_MAX_F64       : DS_1A1D_NORET_mc<"ds_max_f64", AVLdSt_64>;
 
-defm DS_ADD_RTN_U32   : DS_1A1D_RET_mc<"ds_add_rtn_u32", VGPR_32>;
+defm DS_ADD_RTN_U32   : DS_1A1D_RET_mc<"ds_add_rtn_u32">;
 
 let SubtargetPredicate = HasLDSFPAtomicAddF32 in {
-defm DS_ADD_RTN_F32   : DS_1A1D_RET_mc<"ds_add_rtn_f32", VGPR_32>;
-}
-defm DS_SUB_RTN_U32   : DS_1A1D_RET_mc<"ds_sub_rtn_u32", VGPR_32>;
-defm DS_RSUB_RTN_U32  : DS_1A1D_RET_mc<"ds_rsub_rtn_u32", VGPR_32>;
-defm DS_INC_RTN_U32   : DS_1A1D_RET_mc<"ds_inc_rtn_u32", VGPR_32>;
-defm DS_DEC_RTN_U32   : DS_1A1D_RET_mc<"ds_dec_rtn_u32", VGPR_32>;
-defm DS_MIN_RTN_I32   : DS_1A1D_RET_mc<"ds_min_rtn_i32", VGPR_32>;
-defm DS_MAX_RTN_I32   : DS_1A1D_RET_mc<"ds_max_rtn_i32", VGPR_32>;
-defm DS_MIN_RTN_U32   : DS_1A1D_RET_mc<"ds_min_rtn_u32", VGPR_32>;
-defm DS_MAX_RTN_U32   : DS_1A1D_RET_mc<"ds_max_rtn_u32", VGPR_32>;
-defm DS_AND_RTN_B32   : DS_1A1D_RET_mc<"ds_and_rtn_b32", VGPR_32>;
-defm DS_OR_RTN_B32    : DS_1A1D_RET_mc<"ds_or_rtn_b32", VGPR_32>;
-defm DS_XOR_RTN_B32   : DS_1A1D_RET_mc<"ds_xor_rtn_b32", VGPR_32>;
-defm DS_MSKOR_RTN_B32 : DS_1A2D_RET_mc<"ds_mskor_rtn_b32", VGPR_32>;
-defm DS_CMPST_RTN_B32 : DS_1A2D_RET_mc<"ds_cmpst_rtn_b32", VGPR_32>;
-defm DS_CMPST_RTN_F32 : DS_1A2D_RET_mc<"ds_cmpst_rtn_f32", VGPR_32>;
-defm DS_MIN_RTN_F32   : DS_1A1D_RET_mc<"ds_min_rtn_f32", VGPR_32>;
-defm DS_MAX_RTN_F32   : DS_1A1D_RET_mc<"ds_max_rtn_f32", VGPR_32>;
+defm DS_ADD_RTN_F32   : DS_1A1D_RET_mc<"ds_add_rtn_f32">;
+}
+defm DS_SUB_RTN_U32   : DS_1A1D_RET_mc<"ds_sub_rtn_u32">;
+defm DS_RSUB_RTN_U32  : DS_1A1D_RET_mc<"ds_rsub_rtn_u32">;
+defm DS_INC_RTN_U32   : DS_1A1D_RET_mc<"ds_inc_rtn_u32">;
+defm DS_DEC_RTN_U32   : DS_1A1D_RET_mc<"ds_dec_rtn_u32">;
+defm DS_MIN_RTN_I32   : DS_1A1D_RET_mc<"ds_min_rtn_i32">;
+defm DS_MAX_RTN_I32   : DS_1A1D_RET_mc<"ds_max_rtn_i32">;
+defm DS_MIN_RTN_U32   : DS_1A1D_RET_mc<"ds_min_rtn_u32">;
+defm DS_MAX_RTN_U32   : DS_1A1D_RET_mc<"ds_max_rtn_u32">;
+defm DS_AND_RTN_B32   : DS_1A1D_RET_mc<"ds_and_rtn_b32">;
+defm DS_OR_RTN_B32    : DS_1A1D_RET_mc<"ds_or_rtn_b32">;
+defm DS_...
[truncated]

let EncoderMethod = "getMachineOpValueT16";
}

// TODO: These cases should use default target alignment
Copy link
Contributor

Choose a reason for hiding this comment

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

These are redundant with the VGPRSrc* ones above. And those actually are used in real instructions so they need the DecoderMethod. Can you use those, and add your types to that pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to replace the VGPRSrc* cases with these. These are also used for result operands, so should have a more general name. I don't think the DecoderMethods is actually needed in any of these cases and the default one should work.

We do have different decoders for AV_* cases, which differ between source and results so may need distinguished Src and Dst

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though even the AV src decoders can probably be removed after I finish using explicit opcodes for the AGPR cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the DecoderMethods set on the VGPRSrc* ops are just explicitly setting it to default value tablegen computes for the RegClass

Copy link
Contributor

Choose a reason for hiding this comment

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

All of the DecoderMethods set on the VGPRSrc* ops are just explicitly setting it to default value tablegen computes for the RegClass

In downstream, they are not superfluous.

Copy link
Contributor

Choose a reason for hiding this comment

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

I will take a shot at unifying these. I agree the name VGPROp_ is better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Base automatically changed from users/arsenm/amdgpu/define-agpr-variants-ds-write2-insts to main September 4, 2025 00:13
Start stripping out the uses of getLdStRegisterOperand. This
added a confusing level of indirection where the class at the
definition point was not the actual class used. This was also
pulling in the AV class usage for targets where it isn't
relevant. This was also inflexible for special cases.

Also fixes using default arguments which only served to wrap the
class argument in a RegisterOperand.

This should be done for all the memory instructions.
@arsenm arsenm force-pushed the users/arsenm/amdgpu/use-RegisterOperand-ds-inst-definitions branch from 7ffe987 to 284b8e4 Compare September 4, 2025 03:25
@arsenm arsenm merged commit c5a8841 into main Sep 4, 2025
9 checks passed
@arsenm arsenm deleted the users/arsenm/amdgpu/use-RegisterOperand-ds-inst-definitions branch September 4, 2025 05: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.

5 participants