[AMDGPU][ISel][GFX12] Restrict scalar subword loads to PAL

jmmartinez · jmmartinez · commit d0f835872528 · 2024-11-26T09:50:34.000+01:00
On gfx12, s_buffer_load_(i/u)(8/16) have a hw-bug that is triggered
when:
* the stride is not a multiple of 4, or
* the stride is 0 and the num-records is not a multiple of 4

At the moment, these instructions are only generated for PAL.
But in this case, it is guaranteed that the buffers stride/num-records
are aligned to 4.

This patch prevents the emission of scalar subword loads to PAL, where
the bug would never be triggered, and avoid it in HSA (where it could be
triggered, but it's not used).

Solves SWDEV-498239
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
@@ -6803,13 +6803,38 @@ bool AMDGPULegalizerInfo::legalizeSBufferLoad(LegalizerHelper &Helper,
   unsigned Size = Ty.getSizeInBits();
   MachineFunction &MF = B.getMF();
   unsigned Opc = 0;
+
+  const unsigned MemSize = (Size + 7) / 8;
+  const Align MemAlign = B.getDataLayout().getABITypeAlign(
+      getTypeForLLT(Ty, MF.getFunction().getContext()));
+
+  // FIXME: When intrinsic definition is fixed, this should have an MMO already.
+  MachineMemOperand *MMO = MF.getMachineMemOperand(
+      MachinePointerInfo(),
+      MachineMemOperand::MOLoad | MachineMemOperand::MODereferenceable |
+          MachineMemOperand::MOInvariant,
+      MemSize, MemAlign);
+
   if (Size < 32 && ST.hasScalarSubwordLoads()) {
     assert(Size == 8 || Size == 16);
+    if (!ST.hasScalarSubwordBufferLoads()) {
+      // fallback to S_BUFFER_LOAD_UBYTE/USHORT
+      MI.getOperand(1).setIntrinsicID(Intrinsic::amdgcn_raw_buffer_load);
+
+      Register Zero = B.buildConstant(S32, 0).getReg(0);
+      MI.insert(MI.operands_begin() + 4,
+                MachineOperand::CreateReg(Zero, false));
+
+      MI.addMemOperand(MF, MMO);
+      Observer.changedInstr(MI);
+      return true;
+    }
+
     Opc = Size == 8 ? AMDGPU::G_AMDGPU_S_BUFFER_LOAD_UBYTE
                     : AMDGPU::G_AMDGPU_S_BUFFER_LOAD_USHORT;
     // The 8-bit and 16-bit scalar buffer load instructions have 32-bit
     // destination register.
-    Dst = B.getMRI()->createGenericVirtualRegister(LLT::scalar(32));
+    Dst = B.getMRI()->createGenericVirtualRegister(S32);
   } else {
     Opc = AMDGPU::G_AMDGPU_S_BUFFER_LOAD;
     Dst = OrigDst;
@@ -6834,15 +6859,6 @@ bool AMDGPULegalizerInfo::legalizeSBufferLoad(LegalizerHelper &Helper,
   MI.setDesc(B.getTII().get(Opc));
   MI.removeOperand(1); // Remove intrinsic ID
 
-  // FIXME: When intrinsic definition is fixed, this should have an MMO already.
-  const unsigned MemSize = (Size + 7) / 8;
-  const Align MemAlign = B.getDataLayout().getABITypeAlign(
-      getTypeForLLT(Ty, MF.getFunction().getContext()));
-  MachineMemOperand *MMO = MF.getMachineMemOperand(
-      MachinePointerInfo(),
-      MachineMemOperand::MOLoad | MachineMemOperand::MODereferenceable |
-          MachineMemOperand::MOInvariant,
-      MemSize, MemAlign);
   MI.addMemOperand(MF, MMO);
   if (Dst != OrigDst) {
     MI.getOperand(0).setReg(Dst);
diff --git a/llvm/test/CodeGen/AMDGPU/gfx12_scalar_subword_loads.ll b/llvm/test/CodeGen/AMDGPU/gfx12_scalar_subword_loads.ll
@@ -428,14 +428,6 @@ define amdgpu_ps void @s_buffer_load_byte_imm_offset(<4 x i32> inreg %src, ptr a
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_byte_imm_offset:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_i8 s0, s[0:3], 0x4
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_byte_imm_offset:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_i8 s0, s[0:3], 0x4
@@ -459,14 +451,6 @@ define amdgpu_ps void @s_buffer_load_byte_sgpr(<4 x i32> inreg %src, ptr addrspa
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_byte_sgpr:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_i8 s0, s[0:3], s4 offset:0x0
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_byte_sgpr:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_i8 s0, s[0:3], s4 offset:0x0
@@ -490,14 +474,6 @@ define amdgpu_ps void @s_buffer_load_byte_sgpr_or_imm_offset(<4 x i32> inreg %sr
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_byte_sgpr_or_imm_offset:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_i8 s0, s[0:3], s4 offset:0x64
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_byte_sgpr_or_imm_offset:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_i8 s0, s[0:3], s4 offset:0x64
@@ -535,16 +511,6 @@ define amdgpu_ps void @s_buffer_load_ubyte_imm_offset(<4 x i32> inreg %src, ptr
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_ubyte_imm_offset:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_u8 s0, s[0:3], 0x4
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    s_and_b32 s0, s0, 0xff
-; GISEL-NEXT:    s_delay_alu instid0(SALU_CYCLE_1)
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_ubyte_imm_offset:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_u8 s0, s[0:3], 0x4
@@ -570,16 +536,6 @@ define amdgpu_ps void @s_buffer_load_ubyte_sgpr(<4 x i32> inreg %src, ptr addrsp
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_ubyte_sgpr:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_u8 s0, s[0:3], s4 offset:0x0
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    s_and_b32 s0, s0, 0xff
-; GISEL-NEXT:    s_delay_alu instid0(SALU_CYCLE_1)
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_ubyte_sgpr:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_u8 s0, s[0:3], s4 offset:0x0
@@ -605,16 +561,6 @@ define amdgpu_ps void @s_buffer_load_ubyte_sgpr_or_imm_offset(<4 x i32> inreg %s
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_ubyte_sgpr_or_imm_offset:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_u8 s0, s[0:3], s4 offset:0x64
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    s_and_b32 s0, s0, 0xff
-; GISEL-NEXT:    s_delay_alu instid0(SALU_CYCLE_1)
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_ubyte_sgpr_or_imm_offset:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_u8 s0, s[0:3], s4 offset:0x64
@@ -654,14 +600,6 @@ define amdgpu_ps void @s_buffer_load_short_imm_offset(<4 x i32> inreg %src, ptr
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_short_imm_offset:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_i16 s0, s[0:3], 0x4
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_short_imm_offset:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_i16 s0, s[0:3], 0x4
@@ -685,14 +623,6 @@ define amdgpu_ps void @s_buffer_load_short_sgpr(<4 x i32> inreg %src, ptr addrsp
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_short_sgpr:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_i16 s0, s[0:3], s4 offset:0x0
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_short_sgpr:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_i16 s0, s[0:3], s4 offset:0x0
@@ -716,14 +646,6 @@ define amdgpu_ps void @s_buffer_load_short_sgpr_or_imm_offset(<4 x i32> inreg %s
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_short_sgpr_or_imm_offset:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_i16 s0, s[0:3], s4 offset:0x64
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_short_sgpr_or_imm_offset:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_i16 s0, s[0:3], s4 offset:0x64
@@ -761,16 +683,6 @@ define amdgpu_ps void @s_buffer_load_ushort_imm_offset(<4 x i32> inreg %src, ptr
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_ushort_imm_offset:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_u16 s0, s[0:3], 0x4
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    s_and_b32 s0, s0, 0xffff
-; GISEL-NEXT:    s_delay_alu instid0(SALU_CYCLE_1)
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_ushort_imm_offset:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_u16 s0, s[0:3], 0x4
@@ -796,16 +708,6 @@ define amdgpu_ps void @s_buffer_load_ushort_sgpr(<4 x i32> inreg %src, ptr addrs
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_ushort_sgpr:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_u16 s0, s[0:3], s4 offset:0x0
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    s_and_b32 s0, s0, 0xffff
-; GISEL-NEXT:    s_delay_alu instid0(SALU_CYCLE_1)
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_ushort_sgpr:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_u16 s0, s[0:3], s4 offset:0x0
@@ -831,16 +733,6 @@ define amdgpu_ps void @s_buffer_load_ushort_sgpr_or_imm_offset(<4 x i32> inreg %
 ; DAG-DEFAULT-NEXT:    global_store_b32 v[0:1], v2, off
 ; DAG-DEFAULT-NEXT:    s_endpgm
 ;
-; GISEL-LABEL: s_buffer_load_ushort_sgpr_or_imm_offset:
-; GISEL:       ; %bb.0: ; %main_body
-; GISEL-NEXT:    s_buffer_load_u16 s0, s[0:3], s4 offset:0x64
-; GISEL-NEXT:    s_wait_kmcnt 0x0
-; GISEL-NEXT:    s_and_b32 s0, s0, 0xffff
-; GISEL-NEXT:    s_delay_alu instid0(SALU_CYCLE_1)
-; GISEL-NEXT:    v_mov_b32_e32 v2, s0
-; GISEL-NEXT:    global_store_b32 v[0:1], v2, off
-; GISEL-NEXT:    s_endpgm
-;
 ; DAG-PAL-LABEL: s_buffer_load_ushort_sgpr_or_imm_offset:
 ; DAG-PAL:       ; %bb.0: ; %main_body
 ; DAG-PAL-NEXT:    s_buffer_load_u16 s0, s[0:3], s4 offset:0x64
