[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. In D79744#2069962 , @jdoerfert wrote: > In D79744#2069786 , @arsenm wrote: > > > In D79744#2069774 , @arsenm wrote: > > > > > I think this is

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

jdoerfert added a comment. In D79744#2069786 , @arsenm wrote: > In D79744#2069774 , @arsenm wrote: > > > I think this is converging to adding a new IR attribute that essentially > > just provides the pointee type

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2069774 , @arsenm wrote: > I think this is converging to adding a new IR attribute that essentially just > provides the pointee type for ABI purposes. I guess my name ideas for this > would be "indirect", "value",

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2069620 , @rjmccall wrote: > In D79744#2069324 , @arsenm wrote: > > > In D79744#2050498 , @rjmccall > > wrote: > > > > > > For the purpose

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. In D79744#2069324 , @arsenm wrote: > In D79744#2050498 , @rjmccall wrote: > > > > For the purpose here, only the callee exists. This is essentially a > > > freestanding function, the

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2050498 , @rjmccall wrote: > > For the purpose here, only the callee exists. This is essentially a > > freestanding function, the entry point to the program. > > I'm definitely not going to let you add a new "generic"

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2047788 , @jdoerfert wrote: > In D79744#2047482 , @arsenm wrote: > > > For the purpose here, only the callee exists. This is essentially a > > freestanding function, the entry

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. In D79744#2047482 , @arsenm wrote: > In D79744#2040731 , @rjmccall wrote: > > > In D79744#2040434 , @jdoerfert > > wrote: > > > > > In

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

jdoerfert added a comment. In D79744#2047482 , @arsenm wrote: > For the purpose here, only the callee exists. This is essentially a > freestanding function, the entry point to the program. There is no caller > function, and in the future I would like to

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2040731 , @rjmccall wrote: > In D79744#2040434 , @jdoerfert wrote: > > > In D79744#2040380 , @rjmccall > > wrote: > > > > > In

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. In D79744#2040434 , @jdoerfert wrote: > In D79744#2040380 , @rjmccall wrote: > > > In D79744#2040348 , @jdoerfert > > wrote: > > > > > Drive by, I

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

jdoerfert added a comment. In D79744#2040380 , @rjmccall wrote: > In D79744#2040348 , @jdoerfert wrote: > > > Drive by, I haven't read the entire history. > > > > In D79744#2040208

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. In D79744#2040348 , @jdoerfert wrote: > Drive by, I haven't read the entire history. > > In D79744#2040208 , @rjmccall wrote: > > > I don't understand why `noalias` is even a concern if

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

jdoerfert added a comment. Drive by, I haven't read the entire history. In D79744#2040208 , @rjmccall wrote: > I don't understand why `noalias` is even a concern if the whole buffer passed > to the kernel is read-only. `noalias` is primarily about

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. I don't understand why `noalias` is even a concern if the whole buffer passed to the kernel is read-only. `noalias` is primarily about proving non-interference, but if you can tell IR that the buffer is read-only, that's a much more powerful statement. Regardless,

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2035283 , @rjmccall wrote: > > A completely different approach: OpenMP has to solve some very similar > problems and just lowers them completely in the frontend; have you considered > just doing that? Kernels need

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. In D79744#2035109 , @arsenm wrote: > In D79744#2030710 , @rjmccall wrote: > > > Okay. So the only real ABI here is the layout of the memory that the > > arguments are actually written

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2035109 , @arsenm wrote: > >> Unfortunately, I think `byval` just doesn't match what you want because of >> the mutability — the frontend *has* to have a copy into a local to get IR >> with correct semantics,

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2030710 , @rjmccall wrote: > Okay. So the only real ABI here is the layout of the memory that the > arguments are actually written into? And that memory needs to be treated as > constant? Yes, the actual kernel ABI

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. Okay. So the only real ABI here is the layout of the memory that the arguments are actually written into? And that memory needs to be treated as constant? Unfortunately, I think `byval` just doesn't match what you want because of the mutability — the frontend *has*

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2030535 , @rjmccall wrote: > In D79744#2030481 , @arsenm wrote: > > > In D79744#2030294 , @rjmccall > > wrote: > > > > > The parameter

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. In D79744#2030481 , @arsenm wrote: > In D79744#2030294 , @rjmccall wrote: > > > The parameter variable is formally considered to be in a particular address > > space, and taking the

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm added a comment. In D79744#2030294 , @rjmccall wrote: > The parameter variable is formally considered to be in a particular address > space, and taking the address of it yields a pointer in that address space. > That can only work for an

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rjmccall added a comment. The parameter variable is formally considered to be in a particular address space, and taking the address of it yields a pointer in that address space. That can only work for an indirect argument if either (1) the address space of the pointer that's actually passed

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

rampitec added a comment. Typo in commit message: "Previously, indirect arguments assumed assumed". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79744/new/ https://reviews.llvm.org/D79744 ___ cfe-commits mailing list

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm updated this revision to Diff 263274. arsenm added a comment. Forgot to commit a new test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79744/new/ https://reviews.llvm.org/D79744 Files: clang/include/clang/CodeGen/CGFunctionInfo.h clang/lib/CodeGen/CGCall.cpp

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

arsenm created this revision. arsenm added reviewers: yaxunl, hliao, jdoerfert, rjmccall, Anastasia, rampitec. Herald added subscribers: kerbowa, nhaehnle, wdng, jvesely. arsenm added parent revisions: D79732: AMDGPU/HIP: Don't replace pointer types in kernel argument structs, D79630: AMDGPU: