[PATCH] D139213: [llvm][CUDA] Make NVVM Reflect pass look inside ptr casting

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: llvm/test/CodeGen/NVPTX/nvvm-reflect-cast.ll:16
+; COMMON-NOT: call i32 @__nvvm_reflect
+  %reflect = tail call i32 bitcast (i32 (i8*)* @__nvvm_reflect to i32 (i8 addrspace(4)*)*)(i8 addrspace(4)* noundef getelementptr inbounds ([12 x i8], [12 x i8] addrspace(4)* @"$str", i64 0, i64 0))
+; SM20: ret i32 200  
----------------
hdelan wrote:
> tra wrote:
> > hdelan wrote:
> > > tra wrote:
> > > > Interesting. 
> > > > 
> > > > Wouldn't it be better/easier to generate ASC(4->0) of the string argument instead of casting the  `__nvvm_reflect` type?
> > > > Or declare an overload of __nvvm_reflect() accepting the pointer in AS(4)?
> > > > 
> > > > I'm OK with this change, but I want to understand the full picture, and have the motivation/context for the change recorded. Otherwise it's not clear why one would need this at all.
> > > This isn't possible when dealing with llvm intrinsics that do not map to clang builtins. When the first `__nvvm_reflect` call occurs in a module, a declaration is made based on how it is called. This make a declaration for AS4, AS1 etc. Then if say `__nvvm_reflect` is called again in the same module from a different AS, ptr casting will be employed so that the existing declaration will work with the new addrspace of the parameter.
> > > 
> > > When using the clang builtin this doesn't happen since use of `__nvvm_reflect` builtin clearly maps to the llvm intrinsic, which isn't tied to a particular address space.
> > > When the first __nvvm_reflect call occurs in a module, a declaration is made based on how it is called
> > 
> > Are you saying that the code relies on implicit declaration of `__nvvm_reflect` and does not provide it explicitly? 
> > What if someone calls `__nvvm_reflect(1)` ?
> > 
> > I do not see how the declaration which would be provided by compiler for  `__nvvm_reflect` if it were a builtin would be different from providing such a declaration in the user code.
> > 
> > > When using the clang builtin this doesn't happen since use of __nvvm_reflect builtin clearly maps to the llvm intrinsic, which isn't tied to a particular address space.
> > 
> > I'm not sure what does it have to do with the intrinsics at all. All you need is a declaration for a function `extern "C" int__nvvm_reflect(const char*);` 
> > https://godbolt.org/z/MoqMYfjcx
> > 
> > My understanding was that the problem you needed to solve is that constant strings lived in AS(4) so the only piece missing is ability of NVVMReflect pass to look through addrspacecast AS(4 or whatever) ->AS(0) which would be wrapping the string constant in your case.
> > 
> > What do I miss?
> And we cannot do any casting within openCL since strings must be in the `__constant` address space
Nothing stops you declaring nvvm_reflect with a `__constant` argument: https://godbolt.org/z/jTbvcsa3a
NVVM reflect does not care as long as we can get the string value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139213