[PATCH] D91928: [nvptx] Skip alloca for read-only byval arguments.

[Michael Liao via Phabricator via llvm-commits]

hliao added a comment.

In D91928#2409971 <https://reviews.llvm.org/D91928#2409971>, @jlebar wrote:

> This looks really simple, which is awesome.  I am enthusiastic.  But I am worried it may not be correct.
>
> AIUI params are special in that they *must* be read from the param address space.  It is illegal to do a generic load of a param.
>
> So this change is correct only if we can guarantee that address space inference will infer the specific address space for all uses of the pointer.
>
> But address space inference is not guaranteed.  For example, you could `select` on two pointers of two different address spaces.  So long as you only ever read from these pointers, the arg can still be marked as ReadOnly.  But with this patch, we'd end up doing a generic load from the param space, which would be illegal.
>
> Take it all with a grain of salt since I've also been out of the game for a while.

`readonly` is marked in the middle-end (function and argument attribute deduction) and way before the backend. That deduction looks through all relevant users including PHI, addrspacecast and calls. I don't believe there's any exception to prove that deduction wrong.
The address space inference here only refers to the one in the backend directly after this argument lowering gpass. Thep helps translate that argument loading into the correct one using parameter space and doesn't help the aforementioned argument attribute deduction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91928