[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

yaxunl added a comment. In HIP, kernels are represented by attribute `global` and not by calling convention in clang. This may be an alternative. Another alternative might be merging amdgpu_kernel and opencl_kernel calling convention since for the same target they are the same. They could be r

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

aaron.ballman added a comment. In D125970#3531673 , @JonChesterfield wrote: > In D125970#3531645 , @aaron.ballman > wrote: > >> In D125970#3527685 , >> @JonChesterfield

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield added a comment. In D125970#3531645 , @aaron.ballman wrote: > In D125970#3527685 , > @JonChesterfield wrote: > >> If it was adding a calling convention, sure - caution warranted. There's no >> l

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

aaron.ballman added a comment. In D125970#3527685 , @JonChesterfield wrote: > If it was adding a calling convention, sure - caution warranted. There's no > llvm change here though, an existing CC is exposed to C++. No change to the > type system either

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield added a comment. If it was adding a calling convention, sure - caution warranted. There's no llvm change here though, an existing CC is exposed to C++. No change to the type system either. I'll propose a patch with some documentation for it if you wish, but it'll just say "For

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

aaron.ballman added a comment. In D125970#3527624 , @aaron.ballman wrote: >> The primary application is for more rapid debugging of the amdgpu backend by >> permuting a C or C++ test file instead of manually updating an IR file. > > Given that this is a

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

aaron.ballman added a comment. > The primary application is for more rapid debugging of the amdgpu backend by > permuting a C or C++ test file instead of manually updating an IR file. Given that this is adding a calling convention, which has significant impacts on our type system: is this use c

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield added a comment. A clangd buildbot (https://lab.llvm.org/buildbot/#/builders/131/builds/27770) failed on this with [ RUN ] SerializationTest.NoCrashOnBadArraySize ==384111==ERROR: ThreadSanitizer failed to allocate 0x1 (65536) bytes of stack depot (error code: 12)

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG83c431fb9e72: [amdgpu] Add amdgpu_kernel calling conv attribute to clang (authored by JonChesterfield). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield added a comment. Unintentionally created this patch against an older version of main and it interacted badly with D124998 on the rebase. Rerunning tests now, and will leave this open for further comments for a little while. Thanks all Reposit

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield updated this revision to Diff 430821. JonChesterfield added a comment. - Fix git merge misfires Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125970/new/ https://reviews.llvm.org/D125970 Files: clang/include/clang/Basic/Attr.td

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield updated this revision to Diff 430820. JonChesterfield added a comment. - Fix git merge misfires Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125970/new/ https://reviews.llvm.org/D125970 Files: clang/include/clang/Basic/Attr.td

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield updated this revision to Diff 430818. JonChesterfield added a comment. - Rebase on main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125970/new/ https://reviews.llvm.org/D125970 Files: clang/include/clang/Basic/Attr.td clang/i

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield added a comment. Thanks for accepting! I'm interested to learn more about how the calling conv works, e.g. if parts of it are implemented in clang and parts of it patched on the fly by opt, but that's downstream of easy access to writing C tests that use it. Repository: rG L

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125970/new/ https://reviews.llvm.org/D125970 __

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield added a comment. Added a codegen test for arg passing. It establishes that most arguments are left alone, but structs passed by value are handled as an addrspace(4) byref. Letting opt -O2 run annotated some argument pointers as being in addrspace(1) which I think is wrong. I ha

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield updated this revision to Diff 430801. JonChesterfield added a comment. - Add O0 arg passing codegen test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125970/new/ https://reviews.llvm.org/D125970 Files: clang/include/clang/Basic/

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield added a comment. OK, so that's a different thing. CUDA/HIP has a bunch of rules about implicitly tagging things with addrspace(1) at the call boundary. I don't think any of that magic should exist for C or C++, the developer gets to spell out the address space stuff they want ex

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

yaxunl added a comment. In D125970#3526053 , @JonChesterfield wrote: > In D125970#3525985 , @yaxunl wrote: > >> need a codegen test to make sure amdgpu_kernel ABI is used in C/C++ for >> functions with this attr

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield added a comment. In D125970#3525985 , @yaxunl wrote: > need a codegen test to make sure amdgpu_kernel ABI is used in C/C++ for > functions with this attribute We've already got tests that check the amdgpu_kernel calling conv is lowered

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

yaxunl added a comment. need a codegen test to make sure amdgpu_kernel ABI is used in C/C++ for functions with this attribute. https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu#L64 may be used as an example. Repository: rG LLVM Github Mo

[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

JonChesterfield created this revision. JonChesterfield added reviewers: arsenm, rampitec, sdesmalen, rjmccall, rnk, aaron.ballman. Herald added subscribers: kosarev, kerbowa, t-tye, Anastasia, tpr, dstuttard, yaxunl, jvesely, kzhuravl. Herald added a project: All. JonChesterfield requested review