[AMDGPU] Support unaligned flat scratch in TLI

[rampitec]

Summary

Adjust SITargetLowering::allowsMisalignedMemoryAccessesImpl for
unaligned flat scratch support. Mostly needed for global isel.

[arsenm]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

1479

Does it really only work if accessed through the flat instructions? I thought it would even through MUBUF

[rampitec]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

1479

Yes, it is only flat scratch. For MUBUF spec says: "Swizzled addressing also requires dword aligned accesses".

[arsenm]

I think there's some missing tests, these test changes look incidental. Can you add some checks to the existing unaligned load/store base tests

[rampitec]

In D93669#2468820, @arsenm wrote:

I think there's some missing tests, these test changes look incidental. Can you add some checks to the existing unaligned load/store base tests

Which tests do you mean? As far as I understand this mostly affects GlobalISel and child patch D93670 contains the actual test, all unaligned cases.

[arsenm]

In D93669#2468858, @rampitec wrote:

In D93669#2468820, @arsenm wrote:

I think there's some missing tests, these test changes look incidental. Can you add some checks to the existing unaligned load/store base tests

Which tests do you mean? As far as I understand this mostly affects GlobalISel and child patch D93670 contains the actual test, all unaligned cases.

This should cover both. I mean unaligned-load-store.ll (and some others, it's annoying how the test names for these aren't consistent and there is redundancy between files)

[rampitec]

In D93669#2468868, @arsenm wrote:

In D93669#2468858, @rampitec wrote:

In D93669#2468820, @arsenm wrote:

I think there's some missing tests, these test changes look incidental. Can you add some checks to the existing unaligned load/store base tests

Which tests do you mean? As far as I understand this mostly affects GlobalISel and child patch D93670 contains the actual test, all unaligned cases.

This should cover both. I mean unaligned-load-store.ll (and some others, it's annoying how the test names for these aren't consistent and there is redundancy between files)

It should not change without flat scratch enabled?

[rampitec]

In D93669#2468871, @rampitec wrote:

In D93669#2468868, @arsenm wrote:

In D93669#2468858, @rampitec wrote:

In D93669#2468820, @arsenm wrote:

I think there's some missing tests, these test changes look incidental. Can you add some checks to the existing unaligned load/store base tests

Which tests do you mean? As far as I understand this mostly affects GlobalISel and child patch D93670 contains the actual test, all unaligned cases.

This should cover both. I mean unaligned-load-store.ll (and some others, it's annoying how the test names for these aren't consistent and there is redundancy between files)

It should not change without flat scratch enabled?

Or do you want me to add flat scratch to the unaligned-load-store.ll?

[arsenm]

In D93669#2468877, @rampitec wrote:

In D93669#2468871, @rampitec wrote:

In D93669#2468868, @arsenm wrote:

In D93669#2468858, @rampitec wrote:

In D93669#2468820, @arsenm wrote:

I think there's some missing tests, these test changes look incidental. Can you add some checks to the existing unaligned load/store base tests

Which tests do you mean? As far as I understand this mostly affects GlobalISel and child patch D93670 contains the actual test, all unaligned cases.

This should cover both. I mean unaligned-load-store.ll (and some others, it's annoying how the test names for these aren't consistent and there is redundancy between files)

It should not change without flat scratch enabled?

Or do you want me to add flat scratch to the unaligned-load-store.ll?

Yes, I think we're missing some testing combinations (not helped by the mess from the old tests)

[rampitec]

Added run line to the unaligned-load-store.ll.

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

if (Size == 128) {
       bool Aligned = Alignment >= Align(8);
       if (IsFast)
         *IsFast = Aligned;
 
       return Aligned;
     }
   }
 
+  if (AddrSpace == AMDGPUAS::PRIVATE_ADDRESS) {
+    bool AlignedBy4 = Alignment >= Align(4);
+    if (IsFast)
+      *IsFast = AlignedBy4;
+
+    return AlignedBy4 ||
+           Subtarget->enableFlatScratch() ||
+           Subtarget->hasUnalignedScratchAccess();
+  }
+
   // FIXME: We have to be conservative here and assume that flat operations
   // will access scratch.  If we had access to the IR function, then we
   // could determine if any private memory was used in the function.
-  if (!Subtarget->hasUnalignedScratchAccess() &&
-      (AddrSpace == AMDGPUAS::PRIVATE_ADDRESS ||
-       AddrSpace == AMDGPUAS::FLAT_ADDRESS)) {
+  if (AddrSpace == AMDGPUAS::FLAT_ADDRESS &&
+      !Subtarget->hasUnalignedScratchAccess()) {
     bool AlignedBy4 = Alignment >= Align(4);
     if (IsFast)
       *IsFast = AlignedBy4;
 
     return AlignedBy4;
   }
 
   if (Subtarget->hasUnalignedBufferAccessEnabled() &&

llvm/test/CodeGen/AMDGPU/chain-hi-to-lo.ll

 ; FLATSCR-NEXT:    s_mov_b32 vcc_hi, 0
 ; FLATSCR-NEXT:    s_waitcnt vmcnt(0)
 ; FLATSCR-NEXT:    scratch_store_short off, v0, vcc_hi offset:6
 ; FLATSCR-NEXT:    global_load_ushort v0, v2, s[0:1] offset:4
 ; FLATSCR-NEXT:    s_mov_b32 vcc_hi, 0
 ; FLATSCR-NEXT:    s_waitcnt vmcnt(0)
 ; FLATSCR-NEXT:    scratch_store_short off, v0, vcc_hi offset:8
 ; FLATSCR-NEXT:    s_mov_b32 vcc_hi, 0
-; FLATSCR-NEXT:    scratch_load_ushort v0, off, vcc_hi offset:4
+; FLATSCR-NEXT:    scratch_load_dword v0, off, vcc_hi offset:4
 ; FLATSCR-NEXT:    s_mov_b32 vcc_hi, 0
-; FLATSCR-NEXT:    scratch_load_ushort v3, off, vcc_hi offset:6
-; FLATSCR-NEXT:    s_mov_b32 vcc_hi, 0
-; FLATSCR-NEXT:    s_waitcnt vmcnt(1)
-; FLATSCR-NEXT:    v_and_b32_e32 v0, 0xffff, v0
-; FLATSCR-NEXT:    s_waitcnt vmcnt(0)
-; FLATSCR-NEXT:    v_mov_b32_e32 v1, v3
-; FLATSCR-NEXT:    scratch_load_short_d16_hi v1, off, vcc_hi offset:8
-; FLATSCR-NEXT:    v_lshl_or_b32 v0, v3, 16, v0
+; FLATSCR-NEXT:    scratch_load_dword v1, off, vcc_hi offset:6
 ; FLATSCR-NEXT:    s_waitcnt vmcnt(0)
 ; FLATSCR-NEXT:    global_store_dwordx2 v2, v[0:1], s[2:3]
 ; FLATSCR-NEXT:    s_endpgm
 entry:
   %loc = alloca [3 x i16], align 2, addrspace(5)
   %loc.0.sroa_cast1 = bitcast [3 x i16] addrspace(5)* %loc to i8 addrspace(5)*
   %tmp = load i16, i16 addrspace(1)* %in, align 2
   %loc.0.sroa_idx = getelementptr inbounds [3 x i16], [3 x i16] addrspace(5)* %loc, i32 0, i32 0

llvm/test/CodeGen/AMDGPU/unaligned-load-store.ll

-; RUN: llc -march=amdgcn -verify-machineinstrs< %s | FileCheck -check-prefix=SI -check-prefix=ALIGNED %s
-; RUN: llc -march=amdgcn -mcpu=bonaire -mattr=+unaligned-access-mode -verify-machineinstrs< %s | FileCheck -check-prefix=SI -check-prefix=UNALIGNED %s
-; RUN: llc -march=amdgcn -mcpu=tonga -mattr=-flat-for-global -verify-machineinstrs< %s | FileCheck -check-prefix=SI -check-prefix=ALIGNED %s
+; RUN: llc -march=amdgcn -verify-machineinstrs< %s | FileCheck -check-prefixes=SI,MUBUF,ALIGNED %s
+; RUN: llc -march=amdgcn -mcpu=bonaire -mattr=+unaligned-access-mode -verify-machineinstrs< %s | FileCheck -check-prefixes=SI,MUBUF,UNALIGNED %s
+; RUN: llc -march=amdgcn -mcpu=tonga -mattr=-flat-for-global -verify-machineinstrs< %s | FileCheck -check-prefixes=SI,MUBUF,ALIGNED %s
+; RUN: llc -march=amdgcn -mcpu=gfx900 -mattr=-flat-for-global -amdgpu-enable-flat-scratch -verify-machineinstrs < %s | FileCheck -check-prefixes=SI,FLATSCR,ALIGNED %s
 
 ; SI-LABEL: {{^}}local_unaligned_load_store_i16:
 ; SI: ds_read_u8
 ; SI: ds_read_u8
 ; SI: ds_write_b8
 ; SI: ds_write_b8
 ; SI: s_endpgm
 define amdgpu_kernel void @local_unaligned_load_store_i16(i16 addrspace(3)* %p, i16 addrspace(3)* %r) #0 {

 
 ; SI: ScratchSize: 0{{$}}
 define amdgpu_kernel void @local_store_align1_v16i8(<16 x i8> addrspace(3)* %out) #0 {
   store <16 x i8> zeroinitializer, <16 x i8> addrspace(3)* %out, align 1
   ret void
 }
 
 ; SI-LABEL: {{^}}private_load_align1_f64:
-; SI: buffer_load_ubyte
-; SI: buffer_load_ubyte
-; SI: buffer_load_ubyte
-; SI: buffer_load_ubyte
-; SI: buffer_load_ubyte
-; SI: buffer_load_ubyte
-; SI: buffer_load_ubyte
-; SI: buffer_load_ubyte
+; MUBUF: buffer_load_ubyte
+; MUBUF: buffer_load_ubyte
+; MUBUF: buffer_load_ubyte
+; MUBUF: buffer_load_ubyte
+; MUBUF: buffer_load_ubyte
+; MUBUF: buffer_load_ubyte
+; MUBUF: buffer_load_ubyte
+; MUBUF: buffer_load_ubyte
+; FLATSCR: scratch_load_dwordx2
 define double @private_load_align1_f64(double addrspace(5)* %in) {
   %x = load double, double addrspace(5)* %in, align 1
   ret double %x
 }
 
 ; SI-LABEL: {{^}}private_store_align1_f64:
-; SI: buffer_store_byte
-; SI: buffer_store_byte
-; SI: buffer_store_byte
-; SI: buffer_store_byte
-; SI: buffer_store_byte
-; SI: buffer_store_byte
-; SI: buffer_store_byte
-; SI: buffer_store_byte
+; MUBUF: buffer_store_byte
+; MUBUF: buffer_store_byte
+; MUBUF: buffer_store_byte
+; MUBUF: buffer_store_byte
+; MUBUF: buffer_store_byte
+; MUBUF: buffer_store_byte
+; MUBUF: buffer_store_byte
+; MUBUF: buffer_store_byte
+; FLATSCR: scratch_store_dwordx2
 define void @private_store_align1_f64(double addrspace(5)* %out, double %x) #0 {
   store double %x, double addrspace(5)* %out, align 1
   ret void
 }
 
 ; SI-LABEL: {{^}}private_load_align4_f64:
-; SI: buffer_load_dword
-; SI: buffer_load_dword
+; MUBUF: buffer_load_dword
+; MUBUF: buffer_load_dword
+; FLATSCR: scratch_load_dwordx2
 define double @private_load_align4_f64(double addrspace(5)* %in) {
   %x = load double, double addrspace(5)* %in, align 4
   ret double %x
 }
 
 ; SI-LABEL: {{^}}private_store_align4_f64:
-; SI: buffer_store_dword
-; SI: buffer_store_dword
+; MUBUF: buffer_store_dword
+; MUBUF: buffer_store_dword
+; FLATSCR: scratch_store_dwordx2
 define void @private_store_align4_f64(double addrspace(5)* %out, double %x) #0 {
   store double %x, double addrspace(5)* %out, align 4
   ret void
 }
 
 ; SI-LABEL: {{^}}private_load_align2_f64:
-; SI: buffer_load_ushort
-; SI: buffer_load_ushort
-; SI: buffer_load_ushort
-; SI: buffer_load_ushort
+; MUBUF: buffer_load_ushort
+; MUBUF: buffer_load_ushort
+; MUBUF: buffer_load_ushort
+; MUBUF: buffer_load_ushort
+; FLATSCR: scratch_load_dwordx2
 define double @private_load_align2_f64(double addrspace(5)* %in) {
   %x = load double, double addrspace(5)* %in, align 2
   ret double %x
 }
 
 ; SI-LABEL: {{^}}private_store_align2_f64:
-; SI: buffer_store_short
-; SI: buffer_store_short
-; SI: buffer_store_short
-; SI: buffer_store_short
+; MUBUF: buffer_store_short
+; MUBUF: buffer_store_short
+; MUBUF: buffer_store_short
+; MUBUF: buffer_store_short
+; FLATSCR: scratch_store_dwordx2
 define void @private_store_align2_f64(double addrspace(5)* %out, double %x) #0 {
   store double %x, double addrspace(5)* %out, align 2
   ret void
 }
 
 ; Should not merge this to a dword store
 define amdgpu_kernel void @global_store_2xi16_align2(i16 addrspace(1)* %p, i16 addrspace(1)* %r) #0 {
   %gep.r = getelementptr i16, i16 addrspace(1)* %r, i64 1

llvm/test/Transforms/LoadStoreVectorizer/AMDGPU/adjust-alloca-alignment.ll

 ; RUN: opt -S -load-store-vectorizer --mcpu=hawaii -mattr=-unaligned-access-mode,+max-private-element-size-16 < %s | FileCheck -check-prefix=ALIGNED -check-prefix=ALL %s
 ; RUN: opt -S -load-store-vectorizer --mcpu=hawaii -mattr=+unaligned-access-mode,+unaligned-scratch-access,+max-private-element-size-16 < %s | FileCheck -check-prefix=UNALIGNED -check-prefix=ALL %s
 ; RUN: opt -S -passes='function(load-store-vectorizer)' --mcpu=hawaii -mattr=-unaligned-access-mode,+max-private-element-size-16 < %s | FileCheck -check-prefix=ALIGNED -check-prefix=ALL %s
 ; RUN: opt -S -passes='function(load-store-vectorizer)' --mcpu=hawaii -mattr=+unaligned-access-mode,+unaligned-scratch-access,+max-private-element-size-16 < %s | FileCheck -check-prefix=UNALIGNED -check-prefix=ALL %s
 
 target triple = "amdgcn--"
 target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
 
 ; ALL-LABEL: @load_unknown_offset_align1_i8(
 ; ALL: alloca [128 x i8], align 1
 ; UNALIGNED: load <2 x i8>, <2 x i8> addrspace(5)* %{{[0-9]+}}, align 1{{$}}
-
-; ALIGNED: load i8, i8 addrspace(5)* %ptr0, align 1{{$}}
-; ALIGNED: load i8, i8 addrspace(5)* %ptr1, align 1{{$}}
 define amdgpu_kernel void @load_unknown_offset_align1_i8(i8 addrspace(1)* noalias %out, i32 %offset) #0 {
   %alloca = alloca [128 x i8], align 1, addrspace(5)
   %ptr0 = getelementptr inbounds [128 x i8], [128 x i8] addrspace(5)* %alloca, i32 0, i32 %offset
   %val0 = load i8, i8 addrspace(5)* %ptr0, align 1
   %ptr1 = getelementptr inbounds i8, i8 addrspace(5)* %ptr0, i32 1
   %val1 = load i8, i8 addrspace(5)* %ptr1, align 1
   %add = add i8 %val0, %val1
   store i8 %add, i8 addrspace(1)* %out
define amdgpu_kernel void @load_unknown_offset_align1_i32(i32 addrspace(1)* noalias %out, i32 %offset) #0 {
   %val0 = load i32, i32 addrspace(5)* %ptr0, align 1
   %ptr1 = getelementptr inbounds i32, i32 addrspace(5)* %ptr0, i32 1
   %val1 = load i32, i32 addrspace(5)* %ptr1, align 1
   %add = add i32 %val0, %val1
   store i32 %add, i32 addrspace(1)* %out
   ret void
 }
 
-; FIXME: Should always increase alignment of the load
 ; Make sure alloca alignment isn't decreased
 ; ALL-LABEL: @load_alloca16_unknown_offset_align1_i32(
 ; ALL: alloca [128 x i32], align 16
 
-; UNALIGNED: load <2 x i32>, <2 x i32> addrspace(5)* %{{[0-9]+}}, align 1{{$}}
-; ALIGNED: load <2 x i32>, <2 x i32> addrspace(5)* %{{[0-9]+}}, align 4{{$}}
+; ALL: load <2 x i32>, <2 x i32> addrspace(5)* %{{[0-9]+}}, align 4{{$}}
 define amdgpu_kernel void @load_alloca16_unknown_offset_align1_i32(i32 addrspace(1)* noalias %out, i32 %offset) #0 {
   %alloca = alloca [128 x i32], align 16, addrspace(5)
   %ptr0 = getelementptr inbounds [128 x i32], [128 x i32] addrspace(5)* %alloca, i32 0, i32 %offset
   %val0 = load i32, i32 addrspace(5)* %ptr0, align 1
   %ptr1 = getelementptr inbounds i32, i32 addrspace(5)* %ptr0, i32 1
   %val1 = load i32, i32 addrspace(5)* %ptr1, align 1
   %add = add i32 %val0, %val1
   store i32 %add, i32 addrspace(1)* %out
define amdgpu_kernel void @store_unknown_offset_align1_i32(i32 addrspace(1)* noalias %out, i32 %offset) #0 {
   %ptr0 = getelementptr inbounds [128 x i32], [128 x i32] addrspace(5)* %alloca, i32 0, i32 %offset
   store i32 9, i32 addrspace(5)* %ptr0, align 1
   %ptr1 = getelementptr inbounds i32, i32 addrspace(5)* %ptr0, i32 1
   store i32 10, i32 addrspace(5)* %ptr1, align 1
   ret void
 }
 
 ; ALL-LABEL: @merge_private_store_4_vector_elts_loads_v4i32(
-; ALIGNED: %alloca = alloca [8 x i32], align 4, addrspace(5)
-; ALIGNED: store <4 x i32> <i32 9, i32 1, i32 23, i32 19>, <4 x i32> addrspace(5)* %1, align 4
-
-; UNALIGNED: %alloca = alloca [8 x i32], align 1, addrspace(5)
-; UNALIGNED: store <4 x i32> <i32 9, i32 1, i32 23, i32 19>, <4 x i32> addrspace(5)* %1, align 1
+; ALL: %alloca = alloca [8 x i32], align 4, addrspace(5)
+; ALL: store <4 x i32> <i32 9, i32 1, i32 23, i32 19>, <4 x i32> addrspace(5)* %1, align 4
 define amdgpu_kernel void @merge_private_store_4_vector_elts_loads_v4i32() {
   %alloca = alloca [8 x i32], align 1, addrspace(5)
   %out = bitcast [8 x i32] addrspace(5)* %alloca to i32 addrspace(5)*
   %out.gep.1 = getelementptr i32, i32 addrspace(5)* %out, i32 1
   %out.gep.2 = getelementptr i32, i32 addrspace(5)* %out, i32 2
   %out.gep.3 = getelementptr i32, i32 addrspace(5)* %out, i32 3
 
   store i32 9, i32 addrspace(5)* %out, align 1
   store i32 1, i32 addrspace(5)* %out.gep.1, align 1
   store i32 23, i32 addrspace(5)* %out.gep.2, align 1
   store i32 19, i32 addrspace(5)* %out.gep.3, align 1
   ret void
 }
 
 ; ALL-LABEL: @merge_private_store_4_vector_elts_loads_v4i8(
-; ALIGNED: %alloca = alloca [8 x i8], align 4, addrspace(5)
-; ALIGNED: store <4 x i8> <i8 9, i8 1, i8 23, i8 19>, <4 x i8> addrspace(5)* %1, align 4
-
-; UNALIGNED: %alloca = alloca [8 x i8], align 1, addrspace(5)
-; UNALIGNED: store <4 x i8> <i8 9, i8 1, i8 23, i8 19>, <4 x i8> addrspace(5)* %1, align 1
+; ALL: %alloca = alloca [8 x i8], align 4, addrspace(5)
+; ALL: store <4 x i8> <i8 9, i8 1, i8 23, i8 19>, <4 x i8> addrspace(5)* %1, align 4
 define amdgpu_kernel void @merge_private_store_4_vector_elts_loads_v4i8() {
   %alloca = alloca [8 x i8], align 1, addrspace(5)
   %out = bitcast [8 x i8] addrspace(5)* %alloca to i8 addrspace(5)*
   %out.gep.1 = getelementptr i8, i8 addrspace(5)* %out, i8 1
   %out.gep.2 = getelementptr i8, i8 addrspace(5)* %out, i8 2
   %out.gep.3 = getelementptr i8, i8 addrspace(5)* %out, i8 3
 
   store i8 9, i8 addrspace(5)* %out, align 1
   store i8 1, i8 addrspace(5)* %out.gep.1, align 1
   store i8 23, i8 addrspace(5)* %out.gep.2, align 1
   store i8 19, i8 addrspace(5)* %out.gep.3, align 1
   ret void
 }
 
 ; ALL-LABEL: @merge_private_load_4_vector_elts_loads_v4i32(
-; ALIGNED: %alloca = alloca [8 x i32], align 4, addrspace(5)
-; ALIGNED: load <4 x i32>, <4 x i32> addrspace(5)* %1, align 4
-
-; UNALIGNED: %alloca = alloca [8 x i32], align 1, addrspace(5)
-; UNALIGNED: load <4 x i32>, <4 x i32> addrspace(5)* %1, align 1
+; ALL: %alloca = alloca [8 x i32], align 4, addrspace(5)
+; ALL: load <4 x i32>, <4 x i32> addrspace(5)* %1, align 4
 define amdgpu_kernel void @merge_private_load_4_vector_elts_loads_v4i32() {
   %alloca = alloca [8 x i32], align 1, addrspace(5)
   %out = bitcast [8 x i32] addrspace(5)* %alloca to i32 addrspace(5)*
   %out.gep.1 = getelementptr i32, i32 addrspace(5)* %out, i32 1
   %out.gep.2 = getelementptr i32, i32 addrspace(5)* %out, i32 2
   %out.gep.3 = getelementptr i32, i32 addrspace(5)* %out, i32 3
 
   %load0 = load i32, i32 addrspace(5)* %out, align 1
   %load1 = load i32, i32 addrspace(5)* %out.gep.1, align 1
   %load2 = load i32, i32 addrspace(5)* %out.gep.2, align 1
   %load3 = load i32, i32 addrspace(5)* %out.gep.3, align 1
   ret void
 }
 
 ; ALL-LABEL: @merge_private_load_4_vector_elts_loads_v4i8(
-; ALIGNED: %alloca = alloca [8 x i8], align 4, addrspace(5)
-; ALIGNED: load <4 x i8>, <4 x i8> addrspace(5)* %1, align 4
-
-; UNALIGNED: %alloca = alloca [8 x i8], align 1, addrspace(5)
-; UNALIGNED: load <4 x i8>, <4 x i8> addrspace(5)* %1, align 1
+; ALL: %alloca = alloca [8 x i8], align 4, addrspace(5)
+; ALL: load <4 x i8>, <4 x i8> addrspace(5)* %1, align 4
 define amdgpu_kernel void @merge_private_load_4_vector_elts_loads_v4i8() {
   %alloca = alloca [8 x i8], align 1, addrspace(5)
   %out = bitcast [8 x i8] addrspace(5)* %alloca to i8 addrspace(5)*
   %out.gep.1 = getelementptr i8, i8 addrspace(5)* %out, i8 1
   %out.gep.2 = getelementptr i8, i8 addrspace(5)* %out, i8 2
   %out.gep.3 = getelementptr i8, i8 addrspace(5)* %out, i8 3
 
   %load0 = load i8, i8 addrspace(5)* %out, align 1