[PATCH] D156539: [Clang][CodeGen] `__builtin_alloca`s should care about address spaces too

Mon Jul 31 11:43:21 PDT 2023

rjmccall added a comment.

In D156539#4546834 <https://reviews.llvm.org/D156539#4546834>, @AlexVlx wrote:

> In D156539#4542836 <https://reviews.llvm.org/D156539#4542836>, @rjmccall wrote:
>
>> We should probably write this code to work properly in case we add a target that makes `__builtin_alloca` return a pointer in the private address space.  Could you recover the target AS from the type of the expression instead of assuming `LangAS::Default`?
>
> I believe it should work as written (it is possible that I misunderstand the objection, case in which I do apologise). The issue is precisely that for some targets (amdgcn being one) `alloca` returns a pointer to private, which doesn't compose with the default AS being unspecified / nonexistent, for some languages (e.g. HIP,  C/C++ etc.). For the OCL case @arsenm mentions, I believe that we make a choice based on `LangOpts.OpenCLGenericAddressSpace`, and the code above would only be valid from the OCL language perspective iff that is true. I've put together a short Godbolt that captures these (and the bug the patch should fix, please see the bitcasts between ASes in the non-OCL case): https://gcc.godbolt.org/z/sYGK76zqv.

An address space conversion is required in IR if there is a difference between the address space of the pointer type formally returned by the call to `__builtin_alloca` and the address space produced by the `alloca` operation in IR.  If Sema has set the type of `__builtin_alloca` to formally return something in the stack address space, no conversion is required.  What I'm saying that I'd like you to not directly refer to `LangAS::Default` in this code, because you're assuming that `__builtin_alloca` is always returning a pointer that's formally in `LangAS::Default`.  Please recover the target address space from the type of the expression.

Additionally, in IRGen we allow the target to hook address-space conversions; please call `performAddrSpaceCast` instead of directly emitting an `addrspacecast` instruction.

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

https://reviews.llvm.org/D156539