[PATCH] D80237: [hip] Ensure pointer in struct argument has proper `addrspacecast`.

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 19 14:19:07 PDT 2020


tra added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:1339
+      }
+      for (uint64_t i = 0, e = SrcATy->getNumElements(); i < e; ++i) {
+        Address EltPtr = CGF.Builder.CreateConstArrayGEP(Dest, i);
----------------
Is there a limit on array size? We may end up here with potentially unbounded number of spacecasts. Perhaps we need a loop which may later be unrolled, if feasible.


================
Comment at: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu:17
 // HOST: define void @_Z22__device_stub__kernel1Pi(i32* %x)
+// COMMON-LABEL: define amdgpu_kernel void @_Z7kernel1Pi(i32 addrspace(1)*{{.*}} %x.coerce)
+// CHECK:     = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
----------------
Nit: relying on parameter name `%x.coerce` may be rather fragile. Considering that we don't care about the name itself, it may be better to just trim the match after `%x` or even after`%`.


================
Comment at: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu:62-63
+// COMMON-LABEL: define amdgpu_kernel void @_Z7kernel41S(%struct.S.coerce %s.coerce)
+// CHECK:     = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
+// CHECK:     = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
+// CHECK-NOT: = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
----------------
Nit: You could use `CHECK-COUNT-2` : https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-count-directive


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80237/new/

https://reviews.llvm.org/D80237





More information about the cfe-commits mailing list