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

Fri Dec 2 12:09:42 PST 2022

hdelan 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  
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139213