[PATCH] D137154: Adding nvvm_reflect clang builtin

Andrew Savonichev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 1 07:01:52 PDT 2022


asavonic added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsNVVM.td:1581
 def int_nvvm_reflect :
-  Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty], [IntrNoMem], "llvm.nvvm.reflect">;
+  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem], "llvm.nvvm.reflect">,
+  ClangBuiltin<"__nvvm_reflect">;
----------------
hdelan wrote:
> asavonic wrote:
> > What is there a reason to change the intrinsic?
> The clang builtin doesn't link up properly with the llvm intrinsic if `llvm_anyptr_ty` is used. The `llvm_anyptr_ty` suggests to clang to find a general `arg` parameter instead of an `Pointer` which results in a segfault. Changing to `llvm_ptr_ty` fixes the issue
This sounds like a bug in clang. If clang's default lowering for builtins doesn't play well with `llvm_anyptr_ty`, it should be possible to implement it with custom lowering code ( in `CodeGenFunction::EmitNVPTXBuiltinExpr`) without changing the intrinsic. Some intrinsics with anyptr arguments are already implemented with custom lowering (LDG, some atomics).

Changing the intrinsic is problematic for backward compatibility. Overloaded intrinsic from old IR will not be recognized, pointers in non-zero address space cannot be used as arguments directly (must be addrspacecast to generic AS).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137154



More information about the cfe-commits mailing list