[CodeGen] Do not emit call site attributes for LLVM intrinsics. #116881
Conversation
When a function is declared with the `asm(...)` attribute to change the mangled name to reference a LLVM intrinsic, the AST->IR translation for the function declaration already skips any function/parameter attribute that is either deduced from the AST function declaration or implied by language options/target properties. Instead the attributes associated to the LLVM intrinsic function are being generated. When emitting calls to such function declarations, however, call site attributes are emitted based on the AST function declaration or language options/target properties. This can lead to undesired outcomes, e.g. the call site of a LLVM intrinsic marked `convergent` when the language options implies `convergent` by default. This commit fixes the call lowering logic to ignore all attributes in the case the generate call instruction is an intrinsic call, such that the only attributes from the intrinsic declaration will be implicitly considered.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Michele Scandale (michele-scandale) ChangesWhen a function is declared with the When emitting calls to such function declarations, however, call site attributes are emitted based on the AST function declaration or language options/target properties. This commit fixes the call lowering logic to ignore all attributes in the case the generate call instruction is an intrinsic call, such that the only attributes from the intrinsic declaration will be implicitly considered. Full diff: https://github.com/llvm/llvm-project/pull/116881.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 35d495d4dfab82..7af3c676182e5f 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5793,8 +5793,14 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
}
// Apply the attributes and calling convention.
- CI->setAttributes(Attrs);
- CI->setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));
+
+ // If this is a call to an intrinsic, ignore the attributes that would
+ // normally be deduced from the AST function declaration + the default
+ // attributes imposed by the language and/or target.
+ if (!isa<llvm::IntrinsicInst>(CI)) {
+ CI->setAttributes(Attrs);
+ CI->setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));
+ }
// Apply various metadata.
diff --git a/clang/test/CodeGen/llvm-intrinsic-call.c b/clang/test/CodeGen/llvm-intrinsic-call.c
new file mode 100644
index 00000000000000..f71769d652765e
--- /dev/null
+++ b/clang/test/CodeGen/llvm-intrinsic-call.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+float llvm_sin_f32(float) asm("llvm.sin.f32");
+
+float test(float a) {
+ return llvm_sin_f32(a);
+}
+
+// CHECK: call float @llvm.sin.f32(float {{%.+}}){{$}}
+
+// CHECK: declare float @llvm.sin.f32(float) [[ATTRS:#[0-9]+]]
+
+// CHECK: attributes [[ATTRS]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
diff --git a/clang/test/CodeGenCUDA/surface.cu b/clang/test/CodeGenCUDA/surface.cu
index 4106673f3138af..c9e9f1543ec7df 100644
--- a/clang/test/CodeGenCUDA/surface.cu
+++ b/clang/test/CodeGenCUDA/surface.cu
@@ -29,7 +29,7 @@ __attribute__((device)) int suld_2d_zero(surface<void, 2>, int, int) asm("llvm.n
// DEVICE-LABEL: i32 @_Z3fooii(i32 noundef %x, i32 noundef %y)
// DEVICE: call i64 @llvm.nvvm.texsurf.handle.internal.p1(ptr addrspace(1) @surf)
-// DEVICE: call noundef i32 @llvm.nvvm.suld.2d.i32.zero(i64 %{{.*}}, i32 noundef %{{.*}}, i32 noundef %{{.*}})
+// DEVICE: call i32 @llvm.nvvm.suld.2d.i32.zero(i64 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
__attribute__((device)) int foo(int x, int y) {
return suld_2d_zero(surf, x, y);
}
|
|
@llvm/pr-subscribers-clang-codegen Author: Michele Scandale (michele-scandale) ChangesWhen a function is declared with the When emitting calls to such function declarations, however, call site attributes are emitted based on the AST function declaration or language options/target properties. This commit fixes the call lowering logic to ignore all attributes in the case the generate call instruction is an intrinsic call, such that the only attributes from the intrinsic declaration will be implicitly considered. Full diff: https://github.com/llvm/llvm-project/pull/116881.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 35d495d4dfab82..7af3c676182e5f 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5793,8 +5793,14 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
}
// Apply the attributes and calling convention.
- CI->setAttributes(Attrs);
- CI->setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));
+
+ // If this is a call to an intrinsic, ignore the attributes that would
+ // normally be deduced from the AST function declaration + the default
+ // attributes imposed by the language and/or target.
+ if (!isa<llvm::IntrinsicInst>(CI)) {
+ CI->setAttributes(Attrs);
+ CI->setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));
+ }
// Apply various metadata.
diff --git a/clang/test/CodeGen/llvm-intrinsic-call.c b/clang/test/CodeGen/llvm-intrinsic-call.c
new file mode 100644
index 00000000000000..f71769d652765e
--- /dev/null
+++ b/clang/test/CodeGen/llvm-intrinsic-call.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+float llvm_sin_f32(float) asm("llvm.sin.f32");
+
+float test(float a) {
+ return llvm_sin_f32(a);
+}
+
+// CHECK: call float @llvm.sin.f32(float {{%.+}}){{$}}
+
+// CHECK: declare float @llvm.sin.f32(float) [[ATTRS:#[0-9]+]]
+
+// CHECK: attributes [[ATTRS]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
diff --git a/clang/test/CodeGenCUDA/surface.cu b/clang/test/CodeGenCUDA/surface.cu
index 4106673f3138af..c9e9f1543ec7df 100644
--- a/clang/test/CodeGenCUDA/surface.cu
+++ b/clang/test/CodeGenCUDA/surface.cu
@@ -29,7 +29,7 @@ __attribute__((device)) int suld_2d_zero(surface<void, 2>, int, int) asm("llvm.n
// DEVICE-LABEL: i32 @_Z3fooii(i32 noundef %x, i32 noundef %y)
// DEVICE: call i64 @llvm.nvvm.texsurf.handle.internal.p1(ptr addrspace(1) @surf)
-// DEVICE: call noundef i32 @llvm.nvvm.suld.2d.i32.zero(i64 %{{.*}}, i32 noundef %{{.*}}, i32 noundef %{{.*}})
+// DEVICE: call i32 @llvm.nvvm.suld.2d.i32.zero(i64 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
__attribute__((device)) int foo(int x, int y) {
return suld_2d_zero(surf, x, y);
}
|
|
Accessing intrinsics using |
|
Yeah, that's my take as well. Anything we do to make this work better risks being interpreted as treating this as somehow supported, which it absolutely is not. |
|
I understand this is not something recommended. Still there is already code like the one in https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CodeGenModule.cpp#L2887-L2892 which a small trace of intention to do something special about this case. |
Alright. I guess I'll close this PR. |
4 participants
When a function is declared with the
asm(...)attribute to change the mangled name to reference a LLVM intrinsic, the AST->IR translation for the function declaration already skips any function/parameter attribute that is either deduced from the AST function declaration or implied by language options/target properties. Instead the attributes associated to the LLVM intrinsic function are being generated.When emitting calls to such function declarations, however, call site attributes are emitted based on the AST function declaration or language options/target properties.
This can lead to undesired outcomes, e.g. the call site of a LLVM intrinsic marked
convergentwhen the language options impliesconvergentby default.This commit fixes the call lowering logic to ignore all attributes in the case the generate call instruction is an intrinsic call, such that the only attributes from the intrinsic declaration will be implicitly considered.