[AMDGPU] Disable d16 loads/stores to high halves on gfx90a

[krzysz00]

Summary

Testing reveals that, on gfx90a, *_load_short_d16_hi does note
preserve the low-order bits of the destination register, even with
SramECC disabled. To prevent the generation of semantically incorrect
code, disable D16PreservesUnesedBits on this target.

(A more specific test may be needed to account for future gfx90*
targets, but checking for gfx90a instructions was what was in the
existing code)

[rampitec]

Needs test.

llvm/lib/Target/AMDGPU/GCNSubtarget.h

594–596

Typo gff90a.

[krzysz00]

@rampitec What sort of test would be appropriate? I don't know much about this part of LLVM

[rampitec]

In D112635#3090864, @krzysz00 wrote:

@rampitec What sort of test would be appropriate? I don't know much about this part of LLVM

Add run lines and checks for gfx90a to load-hi16.ll and store-hi16.ll.

[arsenm]

Typo in commit message "does note". I don't think we can go ahead and infer the behavior here based on simple experimentation on one system. IIRC there was a combination of firmware and kernel driver fixes to get the correct behavior which you may be missing cc @t-tye

[krzysz00]

• Update tests for the gfx90a changes

[arsenm]

You're probably experiencing a driver issue

[gandhi21299]

Typo in the summary: "D16PreservesUnesedBits" -> "D16PreservesUnusedBits"

[krzysz00]

Confirmed to be a kernel bug, closing.

llvm/lib/Target/AMDGPU/AMDGPU.td

 
 def HasUnpackedD16VMem : Predicate<"Subtarget->hasUnpackedD16VMem()">,
   AssemblerPredicate<(all_of FeatureUnpackedD16VMem)>;
 def HasPackedD16VMem : Predicate<"!Subtarget->hasUnpackedD16VMem()">,
   AssemblerPredicate<(all_of (not FeatureUnpackedD16VMem))>;
 
 def D16PreservesUnusedBits :
   Predicate<"Subtarget->d16PreservesUnusedBits()">,
-  AssemblerPredicate<(all_of FeatureGFX9Insts, (not FeatureSRAMECC))>;
+  AssemblerPredicate<(all_of FeatureGFX9Insts, (not FeatureSRAMECC),
+    (not FeatureGFX90AInsts))>;
 
 def LDSRequiresM0Init : Predicate<"Subtarget->ldsRequiresM0Init()">;
 def NotLDSRequiresM0Init : Predicate<"!Subtarget->ldsRequiresM0Init()">;
 
 def HasDSAddTid : Predicate<"Subtarget->getGeneration() >= AMDGPUSubtarget::GFX9">,
   AssemblerPredicate<(all_of FeatureGFX9Insts)>;
 
 def HasLDSFPAtomicAdd : Predicate<"Subtarget->hasLDSFPAtomicAdd()">,

llvm/lib/Target/AMDGPU/GCNSubtarget.h

bool hasFlatLgkmVMemCountInOrder() const {
     return getGeneration() > GFX9;
   }
 
   bool hasD16LoadStore() const {
     return getGeneration() >= GFX9;
   }
 
   bool d16PreservesUnusedBits() const {
-    return hasD16LoadStore() && !TargetID.isSramEccOnOrAny();
+    // gfx90a's d16 loads don't preserve unused bits
+    return hasD16LoadStore() && !TargetID.isSramEccOnOrAny() &&
+           !hasGFX90AInsts();
   }
 
   bool hasD16Images() const {
     return getGeneration() >= VOLCANIC_ISLANDS;
   }
 
   /// Return if most LDS instructions have an m0 use that require m0 to be
   /// initialized.

llvm/test/CodeGen/AMDGPU/load-hi16.ll

 ; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-sroa=0 -mattr=-promote-alloca -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX900,GFX900-MUBUF %s
 ; RUN: llc -march=amdgcn -mcpu=gfx906 -amdgpu-sroa=0 -mattr=-promote-alloca -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX906,NO-D16-HI %s
+; RUN: llc -march=amdgcn -mcpu=gfx90a -amdgpu-sroa=0 -mattr=-promote-alloca,-sramecc -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,NO-D16-HI %s
 ; RUN: llc -march=amdgcn -mcpu=fiji -amdgpu-sroa=0 -mattr=-promote-alloca -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX803,NO-D16-HI %s
 ; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-sroa=0 -mattr=-promote-alloca -amdgpu-enable-flat-scratch -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX900,GFX900-FLATSCR %s
 
 ; GCN-LABEL: {{^}}load_local_lo_hi_v2i16_multi_use_lo:
 ; GFX900: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX900-NEXT: ds_read_u16 v2, v0
 ; GFX900-DAG: v_mov_b32_e32 [[ZERO:v[0-9]+]], 0
 ; GFX900-DAG: s_waitcnt lgkmcnt(0)

llvm/test/CodeGen/AMDGPU/store-hi16.ll

 ; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-sroa=0 -mattr=-promote-alloca -verify-machineinstrs < %s | FileCheck -allow-deprecated-dag-overlap -check-prefixes=GCN,GFX900,GFX9,GFX900-MUBUF %s
-; RUN: llc -march=amdgcn -mcpu=gfx906 -amdgpu-sroa=0 -mattr=-promote-alloca,+sram-ecc -verify-machineinstrs < %s | FileCheck -allow-deprecated-dag-overlap -check-prefixes=GCN,GFX906,GFX9,NO-D16-HI %s
+; RUN: llc -march=amdgcn -mcpu=gfx906 -amdgpu-sroa=0 -mattr=-promote-alloca,+sramecc -verify-machineinstrs < %s | FileCheck -allow-deprecated-dag-overlap -check-prefixes=GCN,GFX906PLUS,GFX9,NO-D16-HI %s
+; RUN: llc -march=amdgcn -mcpu=gfx90a -amdgpu-sroa=0 -mattr=-promote-alloca,-sramecc -verify-machineinstrs < %s | FileCheck -allow-deprecated-dag-overlap -check-prefixes=GCN,GFX906PLUS,GFX9,NO-D16-HI %s
 ; RUN: llc -march=amdgcn -mcpu=fiji -amdgpu-sroa=0 -mattr=-promote-alloca -verify-machineinstrs < %s | FileCheck -allow-deprecated-dag-overlap -check-prefixes=GCN,GFX803,NO-D16-HI %s
 ; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-sroa=0 -mattr=-promote-alloca -amdgpu-enable-flat-scratch -verify-machineinstrs < %s | FileCheck -allow-deprecated-dag-overlap -check-prefixes=GCN,GFX900,GFX9,GFX900-FLATSCR %s
 
 ; GCN-LABEL: {{^}}store_global_hi_v2i16:
 ; GCN: s_waitcnt
 
 ; GFX900-NEXT: global_store_short_d16_hi v[0:1], v2, off
 
 ; NO-D16-HI-NEXT: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803-NEXT: flat_store_short v[0:1], v2
-; GFX906-NEXT: global_store_short v[0:1], v2, off
+; GFX906PLUS-NEXT: global_store_short v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_v2i16(i16 addrspace(1)* %out, i32 %arg) #0 {
 entry:
   ; FIXME: ABI for pre-gfx9
   %value = bitcast i32 %arg to <2 x i16>
   %hi = extractelement <2 x i16> %value, i32 1
   store i16 %hi, i16 addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_global_hi_v2f16:
 ; GCN: s_waitcnt
 
 ; GFX900-NEXT: global_store_short_d16_hi v[0:1], v2, off
 
 ; NO-D16-HI-NEXT: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803-NEXT: flat_store_short v[0:1], v2
-; GFX906-NEXT: global_store_short v[0:1], v2, off
+; GFX906PLUS-NEXT: global_store_short v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_v2f16(half addrspace(1)* %out, i32 %arg) #0 {
 entry:
   ; FIXME: ABI for pre-gfx9
   %value = bitcast i32 %arg to <2 x half>
   %hi = extractelement <2 x half> %value, i32 1
   store half %hi, half addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_global_hi_i32_shift:
 ; GCN: s_waitcnt
 
 ; GFX900-NEXT: global_store_short_d16_hi v[0:1], v2, off
 
 ; NO-D16-HI-NEXT: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803-NEXT: flat_store_short v[0:1], v2
-; GFX906-NEXT: global_store_short v[0:1], v2, off
+; GFX906PLUS-NEXT: global_store_short v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_i32_shift(i16 addrspace(1)* %out, i32 %value) #0 {
 entry:
   %hi32 = lshr i32 %value, 16
   %hi = trunc i32 %hi32 to i16
   store i16 %hi, i16 addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_global_hi_v2i16_i8:
 ; GCN: s_waitcnt
 
 ; GFX900-NEXT: global_store_byte_d16_hi v[0:1], v2, off
 
 ; NO-D16-HI-NEXT: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803-NEXT: flat_store_byte v[0:1], v2
-; GFX906-NEXT: global_store_byte v[0:1], v2, off
+; GFX906PLUS-NEXT: global_store_byte v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_v2i16_i8(i8 addrspace(1)* %out, i32 %arg) #0 {
 entry:
   %value = bitcast i32 %arg to <2 x i16>
   %hi = extractelement <2 x i16> %value, i32 1
   %trunc = trunc i16 %hi to i8
   store i8 %trunc, i8 addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_global_hi_i8_shift:
 ; GCN: s_waitcnt
 
 ; GFX900-NEXT: global_store_byte_d16_hi v[0:1], v2, off
 
 ; NO-D16-HI-NEXT: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803-NEXT: flat_store_byte v[0:1], v2
-; GFX906-NEXT: global_store_byte v[0:1], v2, off
+; GFX906PLUS-NEXT: global_store_byte v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_i8_shift(i8 addrspace(1)* %out, i32 %value) #0 {
 entry:
   %hi32 = lshr i32 %value, 16
   %hi = trunc i32 %hi32 to i8
   store i8 %hi, i8 addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_global_hi_v2i16_max_offset:
 ; GCN: s_waitcnt
 ; GFX900-NEXT: global_store_short_d16_hi v[0:1], v2, off offset:4094
 
 ; GFX803-DAG: v_add_u32_e32
 ; GFX803-DAG: v_addc_u32_e32
 ; GFX803-DAG: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803: flat_store_short v[0:1], v2{{$}}
 
-; GFX906-NEXT: v_lshrrev_b32_e32 v2, 16, v2
-; GFX906-NEXT: global_store_short v[0:1], v2, off
+; GFX906PLUS-NEXT: v_lshrrev_b32_e32 v2, 16, v2
+; GFX906PLUS-NEXT: global_store_short v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_v2i16_max_offset(i16 addrspace(1)* %out, i32 %arg) #0 {
 entry:
   ; FIXME: ABI for pre-gfx9
   %value = bitcast i32 %arg to <2 x i16>
   %hi = extractelement <2 x i16> %value, i32 1
   %gep = getelementptr inbounds i16, i16 addrspace(1)* %out, i64 2047
   store i16 %hi, i16 addrspace(1)* %gep
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_global_hi_v2i16_min_offset:
 ; GCN: s_waitcnt
 ; GFX900-NEXT: global_store_short_d16_hi v[0:1], v2, off offset:-4096{{$}}
 
 ; GFX803-DAG: v_add_u32_e32
 ; GFX803-DAG: v_addc_u32_e32
 ; GFX803-DAG: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803: flat_store_short v[0:1], v{{[0-9]$}}
 
-; GFX906-NEXT: v_lshrrev_b32_e32 v2, 16, v2
-; GFX906-NEXT: global_store_short v[0:1], v2, off
+; GFX906PLUS-NEXT: v_lshrrev_b32_e32 v2, 16, v2
+; GFX906PLUS-NEXT: global_store_short v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_v2i16_min_offset(i16 addrspace(1)* %out, i32 %arg) #0 {
 entry:
   %value = bitcast i32 %arg to <2 x i16>
   %hi = extractelement <2 x i16> %value, i32 1
   %gep = getelementptr inbounds i16, i16 addrspace(1)* %out, i64 -2048
   store i16 %hi, i16 addrspace(1)* %gep
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_global_hi_v2i16_i8_max_offset:
 ; GCN: s_waitcnt
 ; GFX900-NEXT: global_store_byte_d16_hi v[0:1], v2, off offset:4095
 
 ; GFX803-DAG: v_add_u32_e32
 ; GFX803-DAG: v_addc_u32_e32
 ; GFX803-DAG: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803: flat_store_byte v[0:1], v{{[0-9]$}}
 
-; GFX906-NEXT: v_lshrrev_b32_e32 v2, 16, v2
-; GFX906-NEXT: global_store_byte v[0:1], v2, off
+; GFX906PLUS-NEXT: v_lshrrev_b32_e32 v2, 16, v2
+; GFX906PLUS-NEXT: global_store_byte v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_v2i16_i8_max_offset(i8 addrspace(1)* %out, i32 %arg) #0 {
 entry:
   %value = bitcast i32 %arg to <2 x i16>
   %hi = extractelement <2 x i16> %value, i32 1
   %trunc = trunc i16 %hi to i8
   %gep = getelementptr inbounds i8, i8 addrspace(1)* %out, i64 4095
   store i8 %trunc, i8 addrspace(1)* %gep
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_global_hi_v2i16_i8_min_offset:
 ; GCN: s_waitcnt
 ; GFX900-NEXT: global_store_byte_d16_hi v[0:1], v2, off offset:-4095
 
 ; GFX803-DAG: v_add_u32_e32
 ; GFX803-DAG: v_addc_u32_e32
 ; GFX803-DAG: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803: flat_store_byte v[0:1], v{{[0-9]$}}
 
-; GFX906-NEXT: v_lshrrev_b32_e32 v2, 16, v2
-; GFX906-NEXT: global_store_byte v[0:1], v2, off
+; GFX906PLUS-NEXT: v_lshrrev_b32_e32 v2, 16, v2
+; GFX906PLUS-NEXT: global_store_byte v[0:1], v2, off
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_global_hi_v2i16_i8_min_offset(i8 addrspace(1)* %out, i32 %arg) #0 {
 entry:
   %value = bitcast i32 %arg to <2 x i16>
   %hi = extractelement <2 x i16> %value, i32 1
   %trunc = trunc i16 %hi to i8
entry:
   store i8 %hi, i8* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_flat_hi_v2i16_max_offset:
 ; GCN: s_waitcnt
 ; GFX900-NEXT: flat_store_short_d16_hi v[0:1], v2 offset:4094{{$}}
 
-; GFX906-NEXT: v_lshrrev_b32_e32 v2, 16, v2
-; GFX906-NEXT: flat_store_short v[0:1], v2 offset:4094
+; GFX906PLUS-NEXT: v_lshrrev_b32_e32 v2, 16, v2
+; GFX906PLUS-NEXT: flat_store_short v[0:1], v2 offset:4094
 
 ; GFX803-DAG: v_add_u32_e32
 ; GFX803-DAG: v_addc_u32_e32
 ; GFX803-DAG: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803: flat_store_short v[0:1], v2{{$}}
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64

 ; GCN-LABEL: {{^}}store_flat_hi_v2i16_neg_offset:
 ; GCN: s_waitcnt
 ; GFX803: v_add{{(_co)?}}_{{i|u}}32_e32
 ; GFX803: v_addc_u32_e32
 
 ; GFX9-DAG: v_add_co_u32_e32 v{{[0-9]+}}, vcc, 0xfffff802, v
 ; GFX9-DAG: v_addc_co_u32_e32 v{{[0-9]+}}, vcc, -1, v
 
-; GFX906-DAG: v_lshrrev_b32_e32
-; GFX906: flat_store_short v[0:1], v2{{$}}
+; GFX906PLUS-DAG: v_lshrrev_b32_e32
+; GFX906PLUS: flat_store_short v[0:1], v2{{$}}
 
 ; GFX900-NEXT: flat_store_short_d16_hi v[0:1], v2{{$}}
 ; GFX803: flat_store_short v[0:1], v2{{$}}
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_flat_hi_v2i16_neg_offset(i16* %out, i32 %arg) #0 {
 entry:
   %value = bitcast i32 %arg to <2 x i16>
   %hi = extractelement <2 x i16> %value, i32 1
   %gep = getelementptr inbounds i16, i16* %out, i64 -1023
   store i16 %hi, i16* %gep
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_flat_hi_v2i16_i8_max_offset:
 ; GCN: s_waitcnt
 ; GFX900-NEXT: flat_store_byte_d16_hi v[0:1], v2 offset:4095{{$}}
 
 ; GFX803-DAG: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803-DAG: v_add_u32_e32
 ; GFX803-DAG: v_addc_u32_e32
 ; GFX803: flat_store_byte v[0:1], v2{{$}}
 
-; GFX906-NEXT: v_lshrrev_b32_e32 v2, 16, v2
-; GFX906-NEXT: flat_store_byte v[0:1], v2 offset:4095{{$}}
+; GFX906PLUS-NEXT: v_lshrrev_b32_e32 v2, 16, v2
+; GFX906PLUS-NEXT: flat_store_byte v[0:1], v2 offset:4095{{$}}
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_flat_hi_v2i16_i8_max_offset(i8* %out, i32 %arg) #0 {
 entry:
   %value = bitcast i32 %arg to <2 x i16>
   %hi = extractelement <2 x i16> %value, i32 1
   %trunc = trunc i16 %hi to i8
   %gep = getelementptr inbounds i8, i8* %out, i64 4095
   store i8 %trunc, i8* %gep
   ret void
 }
 
 ; GCN-LABEL: {{^}}store_flat_hi_v2i16_i8_neg_offset:
 ; GCN: s_waitcnt
 
 ; GFX803-DAG: v_add_u32_e32
 ; GFX803-DAG: v_addc_u32_e32
 
 ; GFX9-DAG: v_add_co_u32_e32 v{{[0-9]+}}, vcc, 0xfffff001, v
 ; GFX9-DAG: v_addc_co_u32_e32 v{{[0-9]+}}, vcc, -1, v{{[0-9]+}}, vcc
 
 ; GFX900-NEXT: flat_store_byte_d16_hi v[0:1], v2{{$}}
 
-; GFX906-DAG: v_lshrrev_b32_e32 v2, 16, v2
-; GFX906: flat_store_byte v[0:1], v2{{$}}
+; GFX906PLUS-DAG: v_lshrrev_b32_e32 v2, 16, v2
+; GFX906PLUS: flat_store_byte v[0:1], v2{{$}}
 
 ; GFX803-DAG: v_lshrrev_b32_e32 v2, 16, v2
 ; GFX803: flat_store_byte v[0:1], v2{{$}}
 
 ; GCN-NEXT: s_waitcnt
 ; GCN-NEXT: s_setpc_b64
 define void @store_flat_hi_v2i16_i8_neg_offset(i8* %out, i32 %arg) #0 {
 entry: