[AMDGPU] Fix an invalid cast in AMDGPULateCodeGenPrepare::visitLoadInst

[shiltian]

Fixes: SWDEV-507695

[shiltian]

• [AMDGPU] Fix an invalid cast in AMDGPULateCodeGenPrepare::visitLoadInst #122494 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Shilei Tian (shiltian)

Changes

Fixes SWDEV-507695.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp (+8-2)
• (added) llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll (+43)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp b/llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp
index 830b50307f837e..589a2ef8b54f09 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp
@@ -464,8 +464,14 @@ bool AMDGPULateCodeGenPrepare::visitLoadInst(LoadInst &LI) {
   NewLd->setMetadata(LLVMContext::MD_range, nullptr);
 
   unsigned ShAmt = Adjust * 8;
-  auto *NewVal = IRB.CreateBitCast(
-      IRB.CreateTrunc(IRB.CreateLShr(NewLd, ShAmt), IntNTy), LI.getType());
+  Value *NewVal = nullptr;
+  if (DL.typeSizeEqualsStoreSize(LI.getType())) {
+    NewVal = IRB.CreateBitCast(
+        IRB.CreateTrunc(IRB.CreateLShr(NewLd, ShAmt), IntNTy), LI.getType());
+  } else {
+    assert(DL.getTypeSizeInBits(LI.getType()) < LdBits);
+    NewVal = IRB.CreateTrunc(IRB.CreateLShr(NewLd, ShAmt), LI.getType());
+  }
   LI.replaceAllUsesWith(NewVal);
   DeadInsts.emplace_back(&LI);
 
diff --git a/llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll b/llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll
new file mode 100644
index 00000000000000..e842a70b0aed65
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/invalid-cast-load-i1.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a %s -o - | FileCheck %s
+
+@g = global ptr null
+
+define amdgpu_kernel void @load_idx_idy(ptr addrspace(4) %disp) {
+; CHECK-LABEL: load_idx_idy:
+; CHECK:       ; %bb.0: ; %entry
+; CHECK-NEXT:    s_load_dword s2, s[4:5], 0x4
+; CHECK-NEXT:    s_load_dwordx2 s[0:1], s[8:9], 0x0
+; CHECK-NEXT:    v_mov_b32_e32 v0, 0
+; CHECK-NEXT:    s_waitcnt lgkmcnt(0)
+; CHECK-NEXT:    s_lshr_b32 s2, s2, 16
+; CHECK-NEXT:    s_bfe_i64 s[2:3], s[2:3], 0x10000
+; CHECK-NEXT:    s_lshl_b64 s[2:3], s[2:3], 6
+; CHECK-NEXT:    s_add_u32 s0, s0, s2
+; CHECK-NEXT:    s_addc_u32 s1, s1, s3
+; CHECK-NEXT:    global_load_ubyte v2, v0, s[0:1] offset:4
+; CHECK-NEXT:    s_getpc_b64 s[0:1]
+; CHECK-NEXT:    s_add_u32 s0, s0, g@gotpcrel32@lo+4
+; CHECK-NEXT:    s_addc_u32 s1, s1, g@gotpcrel32@hi+12
+; CHECK-NEXT:    s_load_dwordx2 s[0:1], s[0:1], 0x0
+; CHECK-NEXT:    s_waitcnt lgkmcnt(0)
+; CHECK-NEXT:    v_pk_mov_b32 v[0:1], s[0:1], s[0:1] op_sel:[0,1]
+; CHECK-NEXT:    s_waitcnt vmcnt(0)
+; CHECK-NEXT:    flat_store_byte v[0:1], v2
+; CHECK-NEXT:    s_endpgm
+entry:
+  %disp1 = tail call ptr addrspace(4) @llvm.amdgcn.dispatch.ptr()
+  %gep_y = getelementptr i8, ptr addrspace(4) %disp1, i64 6
+  %L = load i1, ptr addrspace(4) %gep_y, align 1
+  %idxprom = sext i1 %L to i64
+  %gep0 = getelementptr <32 x i16>, ptr addrspace(4) %disp, i64 %idxprom
+  %gep1 = getelementptr i8, ptr addrspace(4) %gep0, i64 4
+  %L1 = load i8, ptr addrspace(4) %gep1
+  store i8 %L1, ptr @g
+  ret void
+}
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare noundef nonnull align 4 ptr addrspace(4) @llvm.amdgcn.dispatch.ptr() #0
+
+attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

[arsenm]

Should be able to use a pointer argument and avoid the special intrinsic. Also can you merge this in with an existing test for this function?

[shiltian]

Should be able to use a pointer argument and avoid the special intrinsic.

The current version was from llvm-reduce. I did try to get rid of the intrinsic but failed to do so, then I didn't dig more into that.

Also can you merge this in with an existing test for this function?

I'm not sure which existing test can be a good fit for this one.

[arsenm]

You probably missed adding the dereferenceable and align attributes to the pointer, which are implied by the intrinsic declaration

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/11861

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Input file: /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
Output file (aarch64): /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
Loaded 3 functions from DWARF.
Loaded 3 functions from symbol table.
warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
addr=0x0000000000000268, file=  2, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000001
addr=0x0000000000000248, file=  1, line=  5
addr=0x0000000000000254, file=  1, line=  7
addr=0x0000000000000258, file=  1, line=  9
addr=0x000000000000025c, file=  1, line=  8
addr=0x0000000000000260, file=  1, line= 11
addr=0x0000000000000264, file=  1, line= 10
...