[PATCH] D118229: [AMDGPUHSAMetadataStreamer] Do not assume ABI alignment for pointers

[Brian Sumner via Phabricator via llvm-commits]

b-sumner added a comment.

In D118229#3291464 <https://reviews.llvm.org/D118229#3291464>, @arsenm wrote:

> In D118229#3291454 <https://reviews.llvm.org/D118229#3291454>, @yaxunl wrote:
>
>> In D118229#3290769 <https://reviews.llvm.org/D118229#3290769>, @arsenm wrote:
>>
>>> In D118229#3290747 <https://reviews.llvm.org/D118229#3290747>, @yaxunl wrote:
>>>
>>>> In D118229#3290659 <https://reviews.llvm.org/D118229#3290659>, @arsenm wrote:
>>>>
>>>>> In D118229#3290658 <https://reviews.llvm.org/D118229#3290658>, @kzhuravl wrote:
>>>>>
>>>>>> This change introduces the regression in OpenCL conformance test: basic - kernel_memory_alignment_local. Does it require any corresponding runtime changes?
>>>>>
>>>>> Is clang correctly emitting the align attribute on all these arguments?
>>>>
>>>> clang does not do anything special for alignment of pointer type kernel arg. It assumes the pointee alignment is default 1. https://godbolt.org/z/xs195rKoW
>>>>
>>>> Question is: What OpenCL spec says about kernel arg pointee alignment? @b-sumner @Anastasia
>>>
>>> It should be the ABI type alignment as was used before
>>
>> I doubt OpenCL spec requires alignment of pointer-type kernel argument. I suspect it is part of our own undocumented ABI. If we implement this in clang, it probably goes to TargetABIInfo.
>
> Yes it does, it requires natural alignment for all types which is what that conformance test is checking.

Agreed.  OpenCL requires a alignof(T) to equal sizeof(T).  There are vload/vstore functions to access vectors which are less aligned than their size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118229