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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118229