JonChesterfield added a comment.

In D125970#3531645 <https://reviews.llvm.org/D125970#3531645>, @aaron.ballman 
wrote:

> In D125970#3527685 <https://reviews.llvm.org/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.
>
> This is adding a user-facing calling convention to Clang and it changes the 
> type system as a result. For example, lambda function pointer conversion 
> operators sometimes are generated for each calling convention so that you can 
> form a function pointer of the correct type (this might not be impacted by 
> your change here); there's a specific number of bits for representing the 
> enumeration of calling conventions and this uses one of those bits, etc.

It slightly changes the type system of C++ code in that the calling convention 
was previously only available in opencl / openmp etc. I was under the 
impression that the compiler data representation cost of calling conventions 
was in LLVM and thus pre-paid for the calling convention this gives access to. 
There's the `enum CallingConv ` which has gained a field, I didn't realise that 
was input into something of limited bitwidth.

>> I'll propose a patch with some documentation for it if you wish, but it'll 
>> just say "For ad hoc debugging of the amdgpu backend". Undocumented seems to 
>> state that more clearly.
>
> I continue to question whether we want to support such a calling convention. 
> This does not seem to be generally useful enough to warrant inclusion in 
> Clang. The fact that you'd like to leave it undocumented as that's more clear 
> for users is a pretty good indication that this calling convention doesn't 
> meet the bar for an extension.

Strictly speaking this lets people write a GPU kernel that can execute on 
AMDGPU in freestanding C++. I happen to want to do that for testing LLVM in the 
immediate instance but there's arguably wider applicability. However, it looks 
like how arguments are represented in this calling convention has some 
strangeness (see discussion with Sam above), particularly with regard to 
address spaces.

I can revert this patch if necessary, but it'll force me to continue trying to 
test our compiler through the lens of opencl, and rules out programming the 
hardware without the various specific language front ends. I think that would 
be a sad loss.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125970

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to