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

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

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.



================
Comment at: llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp:166
+    // generate an `alloca`. Cast it into the parameter space and cast it back
+    // to the generic space so that the address space inference could infer the
+    // correct address space.
----------------
nit: s/could/can/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91928