[llvm] [AMDGPU] Fix predicates on FLAT scratch ST/SVS mode Pseudos (PR #85442)

via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 15 11:11:57 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-amdgpu

Author: Jay Foad (jayfoad)

<details>
<summary>Changes</summary>

Definitions like this did not work as intended:

  let is_flat_scratch = 1 in {
    let SubtargetPredicate = HasFlatScratchSVSMode in
    def _SVS : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput, 1, 1>,
               FlatScratchInst<opName, "SVS">;

    let SubtargetPredicate = HasFlatScratchSTMode in
    def _ST  : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput, 0, 0, 0>,
               FlatScratchInst<opName, "ST">;
  }

They tried to override SubtargetPredicate, but then it was overridden
again (back to its default value) by setting is_flat_scratch, which
caused SubtargetPredicate to be recalculated in the base class. (This
patch also removes some overrides of SubtargetPredicate that are
redundant due to being recalculated in the base class.)

Fix this by pushing overrides of is_flat_scratch and is_flat_global "in"
as far as possible. This has the added benefit that there is no need to
override them around groups of Pseudo definitions like this:

let is_flat_global = 1 in {
defm GLOBAL_ATOMIC_CMPSWAP : FLAT_Global_Atomic_Pseudo <"global_atomic_cmpswap",
                               VGPR_32, i32, v2i32, VReg_64>;
...
}

which are plainly Global instructions anyway.

Verified by inspecting the output of TableGen. It seems to be NFC in
practice.


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


1 Files Affected:

- (modified) llvm/lib/Target/AMDGPU/FLATInstructions.td (+69-77) 


``````````diff
diff --git a/llvm/lib/Target/AMDGPU/FLATInstructions.td b/llvm/lib/Target/AMDGPU/FLATInstructions.td
index cc03ee6c35462d..d35efaa85512a4 100644
--- a/llvm/lib/Target/AMDGPU/FLATInstructions.td
+++ b/llvm/lib/Target/AMDGPU/FLATInstructions.td
@@ -239,7 +239,7 @@ class FLAT_Store_Pseudo <string opName, RegisterClass vdataClass,
 }
 
 multiclass FLAT_Global_Load_Pseudo<string opName, RegisterClass regClass, bit HasTiedInput = 0> {
-  let is_flat_global = 1, SubtargetPredicate = HasFlatGlobalInsts in {
+  let is_flat_global = 1 in {
     def "" : FLAT_Load_Pseudo<opName, regClass, HasTiedInput, 1>,
       GlobalSaddrTable<0, opName>;
     def _SADDR : FLAT_Load_Pseudo<opName, regClass, HasTiedInput, 1, 1>,
@@ -276,7 +276,7 @@ multiclass FLAT_Global_Load_AddTid_Pseudo<string opName, RegisterClass regClass,
 }
 
 multiclass FLAT_Global_Store_Pseudo<string opName, RegisterClass regClass> {
-  let is_flat_global = 1, SubtargetPredicate = HasFlatGlobalInsts in {
+  let is_flat_global = 1 in {
     def "" : FLAT_Store_Pseudo<opName, regClass, 1>,
       GlobalSaddrTable<0, opName>;
     def _SADDR : FLAT_Store_Pseudo<opName, regClass, 1, 1>,
@@ -389,6 +389,7 @@ class FLAT_Scratch_Load_Pseudo <string opName, RegisterClass regClass,
      !if(HasTiedOutput, (ins CPol:$cpol, getLdStRegisterOperand<regClass>.ret:$vdst_in),
                         (ins CPol_0:$cpol))),
   " $vdst, "#!if(EnableVaddr, "$vaddr, ", "off, ")#!if(EnableSaddr, "$saddr", "off")#"$offset$cpol"> {
+  let is_flat_scratch = 1;
   let has_data = 0;
   let mayLoad = 1;
   let has_saddr = 1;
@@ -416,6 +417,7 @@ class FLAT_Scratch_Store_Pseudo <string opName, RegisterClass vdataClass, bit En
         (ins vdata_op:$vdata, VGPR_32:$vaddr, flat_offset:$offset, CPol_0:$cpol),
         (ins vdata_op:$vdata, flat_offset:$offset, CPol_0:$cpol)))),
   " "#!if(EnableVaddr, "$vaddr", "off")#", $vdata, "#!if(EnableSaddr, "$saddr", "off")#"$offset$cpol"> {
+  let is_flat_scratch = 1;
   let mayLoad  = 0;
   let mayStore = 1;
   let has_vdst = 0;
@@ -428,37 +430,33 @@ class FLAT_Scratch_Store_Pseudo <string opName, RegisterClass vdataClass, bit En
 }
 
 multiclass FLAT_Scratch_Load_Pseudo<string opName, RegisterClass regClass, bit HasTiedOutput = 0> {
-  let is_flat_scratch = 1 in {
-    def "" : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput>,
-             FlatScratchInst<opName, "SV">;
-    def _SADDR : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput, 1>,
-                 FlatScratchInst<opName, "SS">;
-
-    let SubtargetPredicate = HasFlatScratchSVSMode in
-    def _SVS : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput, 1, 1>,
-               FlatScratchInst<opName, "SVS">;
+  def "" : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput>,
+           FlatScratchInst<opName, "SV">;
+  def _SADDR : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput, 1>,
+               FlatScratchInst<opName, "SS">;
 
-    let SubtargetPredicate = HasFlatScratchSTMode in
-    def _ST  : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput, 0, 0, 0>,
-               FlatScratchInst<opName, "ST">;
-  }
+  let SubtargetPredicate = HasFlatScratchSVSMode in
+  def _SVS : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput, 1, 1>,
+             FlatScratchInst<opName, "SVS">;
+
+  let SubtargetPredicate = HasFlatScratchSTMode in
+  def _ST  : FLAT_Scratch_Load_Pseudo<opName, regClass, HasTiedOutput, 0, 0, 0>,
+             FlatScratchInst<opName, "ST">;
 }
 
 multiclass FLAT_Scratch_Store_Pseudo<string opName, RegisterClass regClass> {
-  let is_flat_scratch = 1 in {
-    def "" : FLAT_Scratch_Store_Pseudo<opName, regClass>,
-             FlatScratchInst<opName, "SV">;
-    def _SADDR : FLAT_Scratch_Store_Pseudo<opName, regClass, 1>,
-                 FlatScratchInst<opName, "SS">;
-
-    let SubtargetPredicate = HasFlatScratchSVSMode in
-    def _SVS : FLAT_Scratch_Store_Pseudo<opName, regClass, 1, 1>,
-               FlatScratchInst<opName, "SVS">;
+  def "" : FLAT_Scratch_Store_Pseudo<opName, regClass>,
+           FlatScratchInst<opName, "SV">;
+  def _SADDR : FLAT_Scratch_Store_Pseudo<opName, regClass, 1>,
+               FlatScratchInst<opName, "SS">;
 
-    let SubtargetPredicate = HasFlatScratchSTMode in
-    def _ST  : FLAT_Scratch_Store_Pseudo<opName, regClass, 0, 0, 0>,
-               FlatScratchInst<opName, "ST">;
-  }
+  let SubtargetPredicate = HasFlatScratchSVSMode in
+  def _SVS : FLAT_Scratch_Store_Pseudo<opName, regClass, 1, 1>,
+             FlatScratchInst<opName, "SVS">;
+
+  let SubtargetPredicate = HasFlatScratchSTMode in
+  def _ST  : FLAT_Scratch_Store_Pseudo<opName, regClass, 0, 0, 0>,
+             FlatScratchInst<opName, "ST">;
 }
 
 class FLAT_Scratch_Load_LDS_Pseudo <string opName, bit EnableSaddr = 0,
@@ -583,25 +581,27 @@ multiclass FLAT_Global_Atomic_Pseudo_NO_RTN<
   RegisterClass data_rc = vdst_rc,
   RegisterOperand data_op = getLdStRegisterOperand<data_rc>.ret> {
 
-  def "" : FLAT_AtomicNoRet_Pseudo <opName,
-    (outs),
-    (ins VReg_64:$vaddr, data_op:$vdata, flat_offset:$offset, CPol_0:$cpol),
-    " $vaddr, $vdata, off$offset$cpol">,
-    GlobalSaddrTable<0, opName> {
-    let has_saddr = 1;
-    let PseudoInstr = NAME;
-    let FPAtomic = data_vt.isFP;
-  }
+  let is_flat_global = 1 in {
+    def "" : FLAT_AtomicNoRet_Pseudo <opName,
+      (outs),
+      (ins VReg_64:$vaddr, data_op:$vdata, flat_offset:$offset, CPol_0:$cpol),
+      " $vaddr, $vdata, off$offset$cpol">,
+      GlobalSaddrTable<0, opName> {
+      let has_saddr = 1;
+      let PseudoInstr = NAME;
+      let FPAtomic = data_vt.isFP;
+    }
 
-  def _SADDR : FLAT_AtomicNoRet_Pseudo <opName,
-    (outs),
-    (ins VGPR_32:$vaddr, data_op:$vdata, SReg_64:$saddr, flat_offset:$offset, CPol_0:$cpol),
-    " $vaddr, $vdata, $saddr$offset$cpol">,
-    GlobalSaddrTable<1, opName> {
-    let has_saddr = 1;
-    let enabled_saddr = 1;
-    let PseudoInstr = NAME#"_SADDR";
-    let FPAtomic = data_vt.isFP;
+    def _SADDR : FLAT_AtomicNoRet_Pseudo <opName,
+      (outs),
+      (ins VGPR_32:$vaddr, data_op:$vdata, SReg_64:$saddr, flat_offset:$offset, CPol_0:$cpol),
+      " $vaddr, $vdata, $saddr$offset$cpol">,
+      GlobalSaddrTable<1, opName> {
+      let has_saddr = 1;
+      let enabled_saddr = 1;
+      let PseudoInstr = NAME#"_SADDR";
+      let FPAtomic = data_vt.isFP;
+    }
   }
 }
 
@@ -614,24 +614,26 @@ multiclass FLAT_Global_Atomic_Pseudo_RTN<
   RegisterOperand data_op = getLdStRegisterOperand<data_rc>.ret,
   RegisterOperand vdst_op = getLdStRegisterOperand<vdst_rc>.ret> {
 
-  def _RTN : FLAT_AtomicRet_Pseudo <opName,
-    (outs vdst_op:$vdst),
-      (ins VReg_64:$vaddr, data_op:$vdata, flat_offset:$offset, CPol_GLC1:$cpol),
-    " $vdst, $vaddr, $vdata, off$offset$cpol">,
-    GlobalSaddrTable<0, opName#"_rtn"> {
-    let has_saddr = 1;
-    let FPAtomic = data_vt.isFP;
-  }
+  let is_flat_global = 1 in {
+    def _RTN : FLAT_AtomicRet_Pseudo <opName,
+      (outs vdst_op:$vdst),
+        (ins VReg_64:$vaddr, data_op:$vdata, flat_offset:$offset, CPol_GLC1:$cpol),
+      " $vdst, $vaddr, $vdata, off$offset$cpol">,
+      GlobalSaddrTable<0, opName#"_rtn"> {
+      let has_saddr = 1;
+      let FPAtomic = data_vt.isFP;
+    }
 
-  def _SADDR_RTN : FLAT_AtomicRet_Pseudo <opName,
-    (outs vdst_op:$vdst),
-      (ins VGPR_32:$vaddr, data_op:$vdata, SReg_64:$saddr, flat_offset:$offset, CPol_GLC1:$cpol),
-    " $vdst, $vaddr, $vdata, $saddr$offset$cpol">,
-    GlobalSaddrTable<1, opName#"_rtn"> {
-     let has_saddr = 1;
-     let enabled_saddr = 1;
-     let PseudoInstr = NAME#"_SADDR_RTN";
-     let FPAtomic = data_vt.isFP;
+    def _SADDR_RTN : FLAT_AtomicRet_Pseudo <opName,
+      (outs vdst_op:$vdst),
+        (ins VGPR_32:$vaddr, data_op:$vdata, SReg_64:$saddr, flat_offset:$offset, CPol_GLC1:$cpol),
+      " $vdst, $vaddr, $vdata, $saddr$offset$cpol">,
+      GlobalSaddrTable<1, opName#"_rtn"> {
+       let has_saddr = 1;
+       let enabled_saddr = 1;
+       let PseudoInstr = NAME#"_SADDR_RTN";
+       let FPAtomic = data_vt.isFP;
+    }
   }
 }
 
@@ -641,10 +643,8 @@ multiclass FLAT_Global_Atomic_Pseudo<
   ValueType vt,
   ValueType data_vt = vt,
   RegisterClass data_rc = vdst_rc> {
-  let is_flat_global = 1, SubtargetPredicate = HasFlatGlobalInsts in {
-    defm "" : FLAT_Global_Atomic_Pseudo_NO_RTN<opName, vdst_rc, vt, data_vt, data_rc>;
-    defm "" : FLAT_Global_Atomic_Pseudo_RTN<opName, vdst_rc, vt, data_vt, data_rc>;
-  }
+  defm "" : FLAT_Global_Atomic_Pseudo_NO_RTN<opName, vdst_rc, vt, data_vt, data_rc>;
+  defm "" : FLAT_Global_Atomic_Pseudo_RTN<opName, vdst_rc, vt, data_vt, data_rc>;
 }
 
 //===----------------------------------------------------------------------===//
@@ -848,7 +848,6 @@ defm GLOBAL_STORE_DWORD_ADDTID : FLAT_Global_Store_AddTid_Pseudo <"global_store_
 defm GLOBAL_STORE_BYTE_D16_HI  : FLAT_Global_Store_Pseudo <"global_store_byte_d16_hi", VGPR_32>;
 defm GLOBAL_STORE_SHORT_D16_HI : FLAT_Global_Store_Pseudo <"global_store_short_d16_hi", VGPR_32>;
 
-let is_flat_global = 1 in {
 defm GLOBAL_ATOMIC_CMPSWAP : FLAT_Global_Atomic_Pseudo <"global_atomic_cmpswap",
                                VGPR_32, i32, v2i32, VReg_64>;
 
@@ -947,9 +946,6 @@ let SubtargetPredicate = isGFX12Plus in {
   def GLOBAL_WBINV  : FLAT_Global_Invalidate_Writeback<"global_wbinv">;
 } // End SubtargetPredicate = isGFX12Plus
 
-} // End is_flat_global = 1
-
-let SubtargetPredicate = HasFlatScratchInsts in {
 defm SCRATCH_LOAD_UBYTE    : FLAT_Scratch_Load_Pseudo <"scratch_load_ubyte", VGPR_32>;
 defm SCRATCH_LOAD_SBYTE    : FLAT_Scratch_Load_Pseudo <"scratch_load_sbyte", VGPR_32>;
 defm SCRATCH_LOAD_USHORT   : FLAT_Scratch_Load_Pseudo <"scratch_load_ushort", VGPR_32>;
@@ -984,8 +980,6 @@ defm SCRATCH_LOAD_LDS_USHORT : FLAT_Scratch_Load_LDS_Pseudo <"scratch_load_lds_u
 defm SCRATCH_LOAD_LDS_SSHORT : FLAT_Scratch_Load_LDS_Pseudo <"scratch_load_lds_sshort">;
 defm SCRATCH_LOAD_LDS_DWORD  : FLAT_Scratch_Load_LDS_Pseudo <"scratch_load_lds_dword">;
 
-} // End SubtargetPredicate = HasFlatScratchInsts
-
 let SubtargetPredicate = isGFX12Plus in {
   let WaveSizePredicate = isWave32 in {
     defm GLOBAL_LOAD_TR_B128_w32  : FLAT_Global_Load_Pseudo <"global_load_tr_b128_w32", VReg_128>;
@@ -997,7 +991,7 @@ let SubtargetPredicate = isGFX12Plus in {
   }
 } // End SubtargetPredicate = isGFX12Plus
 
-let SubtargetPredicate = isGFX10Plus, is_flat_global = 1 in {
+let SubtargetPredicate = isGFX10Plus in {
   defm GLOBAL_ATOMIC_FCMPSWAP :
     FLAT_Global_Atomic_Pseudo<"global_atomic_fcmpswap", VGPR_32, f32, v2f32, VReg_64>;
   defm GLOBAL_ATOMIC_FMIN :
@@ -1010,9 +1004,8 @@ let SubtargetPredicate = isGFX10Plus, is_flat_global = 1 in {
     FLAT_Global_Atomic_Pseudo<"global_atomic_fmin_x2", VReg_64, f64>;
   defm GLOBAL_ATOMIC_FMAX_X2 :
     FLAT_Global_Atomic_Pseudo<"global_atomic_fmax_x2", VReg_64, f64>;
-} // End SubtargetPredicate = isGFX10Plus, is_flat_global = 1
+} // End SubtargetPredicate = isGFX10Plus
 
-let is_flat_global = 1 in {
 let OtherPredicates = [HasAtomicFaddNoRtnInsts] in
   defm GLOBAL_ATOMIC_ADD_F32 : FLAT_Global_Atomic_Pseudo_NO_RTN <
     "global_atomic_add_f32", VGPR_32, f32
@@ -1029,7 +1022,6 @@ let OtherPredicates = [HasAtomicBufferGlobalPkAddF16Insts] in
   defm GLOBAL_ATOMIC_PK_ADD_F16 : FLAT_Global_Atomic_Pseudo_RTN <
     "global_atomic_pk_add_f16", VGPR_32, v2f16
   >;
-} // End is_flat_global = 1
 
 //===----------------------------------------------------------------------===//
 // Flat Patterns

``````````

</details>


https://github.com/llvm/llvm-project/pull/85442


More information about the llvm-commits mailing list