Created attachment 117515 [details] Code to reproduce the bug clSetKernelArg is erroring out with "CL_INVALID_ARG_SIZE" when trying to pass in a struct as an input. This can be reproduced with the code in the attachment. It can be verified that the size of the struct inside the OpenCL kernel and host are the same by changing "REPRODUCE_BUG" to be 0.
This was tested on Fedora 22 with AMD A10 5800K.
Hello I didn't test your program, but it seems that dim_t on host side should be defined with cl_long instead of long long. See https://www.khronos.org/registry/cl/sdk/1.2/docs/man/xhtml/scalarDataTypes.html
Created attachment 121969 [details] file to reproduce the problem This is still a problem in 11.1.2
I haven't run the program myself either, but it's likely that the module::argument::size value is calculated incorrectly in the compiler glue code, check out the definition of arg_api_size in llvm/invocation.cpp:454, which is likely to be wrong for non-builtin types.
@Fernando, the updated code sample only includes native types inside a struct. But I will look at the file you mention to see if anything fishy is happening.
@fernando, looks like you are right. The code paths do not even consider that users might be sending in custom data types (aka structs / unions).
This simple change should fix the size calculation. However the kernel still receive garbage, but we are working on it since GROMACS have the same problem. Also the serie found at https://lists.freedesktop.org/archives/piglit/2016-February/019074.html is tracking this error diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp index 4d11c24..74c9511 100644 --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp @@ -547,8 +547,10 @@ namespace { module::argument::sign_ext : module::argument::zero_ext); + const unsigned arg_store_alloc = TD.getTypeAllocSize(arg_type); + args.push_back( - module::argument(module::argument::scalar, arg_api_size, + module::argument(module::argument::scalar, arg_store_alloc, target_size, target_align, ext_type)); } }
@serge won't this break the original intent of the padding size ? For example float3 would be broken in this case.
getTypeAllocSize is making the rounding : 00388 uint64_t getTypeAllocSize(Type *Ty) const { 00389 // Round up to the next alignment boundary. 00390 return alignTo(getTypeStoreSize(Ty), getABITypeAlignment(Ty)); 00391 }
Slightly change bug summary.
(In reply to Pavan Yalamanchili from comment #8) > @serge won't this break the original intent of the padding size ? For > example float3 would be broken in this case. No, this would fix float3. 3 vectors are defined to have the size/alignment of the equivalent 4 vector
Is this patch going through into a future release?
(In reply to Pavan Yalamanchili from comment #12) > Is this patch going through into a future release? Structs are not handled correctly even with the patch applied.
Should be fixed in LLLVM 4.0 with: commit 5cb9343f53d83c9a8a33aac5b2ce01672ff02cf3 Author: Matt Arsenault <Matthew.Arsenault@amd.com> Date: Mon Aug 22 19:25:59 2016 +0000 AMDGPU: Handle structs directly in AMDGPUABIInfo Structs are currently handled as pointer + byval, which makes AMDGPU LLVM backend generate incorrect code when structs are used. This patch changes struct argument to be handled directly and without flattening, which Clover (Mesa 3D Gallium OpenCL state tracker) will be able to handle. Flattening would expand the struct to individual elements and pass each as a separate argument, which Clover can not handle. Furthermore, such expansion does not fit the OpenCL programming model which requires to explicitely specify each argument index, size and memory location. Patch by Vedran Miletić git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@279463 91177308-0d34-0410-b5e6-96231b3b80d8 Please recheck.
Fixed for radeonsi in LLVM/Clang 4.0 and for r600 with commit a745be5724ad975e8456500c52f12d7ef2995770 Author: Jan Vesely <jan.vesely@rutgers.edu> Date: Wed Feb 22 15:01:42 2017 +0000 [OpenCL] r600 needs OpenCL kernel calling convention Differential Revision: https://reviews.llvm.org/D30236 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@295843 91177308-0d34-0410-b5e6-96231b3b80d8
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.