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

[Yaxun Liu via Phabricator via llvm-commits]

yaxunl added a comment.

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.

I would suggest reverting this patch since it caused regressions. It should not be committed without corresponding clang change to maintain the ABI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118229