Bug 91556

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