Skip to content

[HLSL] Don't use CreateRuntimeFunction for intrinsics #145334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2025

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jun 23, 2025

HLSL uses CreateRuntimeFunction for three intrinsics. This is pretty unusual thing to do, and doesn't match what the rest of the file does.

I suspect this might be because these are convergent calls, but the intrinsics themselves are already marked convergent, so it's not necessary for clang to manually add the attribute.

This does lose the spir_func CC on the intrinsic declaration, but again, CC should not be relevant to intrinsics at all.

@nikic nikic requested review from bogner and farzonl June 23, 2025 13:58
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. HLSL HLSL Language Support labels Jun 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 23, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang-codegen

Author: Nikita Popov (nikic)

Changes

HLSL uses CreateRuntimeFunction for two intrinsics. This is pretty unusual thing to do, and doesn't match what the rest of the file does.

I suspect this might be because these are convergent calls, but the intrinsics themselves are already marked convergent, so it's not necessary for clang to manually add the attribute.

This does lose the spir_func CC on the intrinsic declaration, but again, CC should not be relevant to intrinsics at all.


Full diff: https://github.com/llvm/llvm-project/pull/145334.diff

3 Files Affected:

  • (modified) clang/lib/CodeGen/CGHLSLBuiltins.cpp (+4-16)
  • (modified) clang/test/CodeGenHLSL/builtins/WaveActiveMax.hlsl (+3-3)
  • (modified) clang/test/CodeGenHLSL/builtins/WaveActiveSum.hlsl (+3-3)
diff --git a/clang/lib/CodeGen/CGHLSLBuiltins.cpp b/clang/lib/CodeGen/CGHLSLBuiltins.cpp
index 2a60a0909c93e..5074a915f2817 100644
--- a/clang/lib/CodeGen/CGHLSLBuiltins.cpp
+++ b/clang/lib/CodeGen/CGHLSLBuiltins.cpp
@@ -676,35 +676,23 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID,
   case Builtin::BI__builtin_hlsl_wave_active_sum: {
     // Due to the use of variadic arguments, explicitly retreive argument
     Value *OpExpr = EmitScalarExpr(E->getArg(0));
-    llvm::FunctionType *FT = llvm::FunctionType::get(
-        OpExpr->getType(), ArrayRef{OpExpr->getType()}, false);
     Intrinsic::ID IID = getWaveActiveSumIntrinsic(
         getTarget().getTriple().getArch(), CGM.getHLSLRuntime(),
         E->getArg(0)->getType());
 
-    // Get overloaded name
-    std::string Name =
-        Intrinsic::getName(IID, ArrayRef{OpExpr->getType()}, &CGM.getModule());
-    return EmitRuntimeCall(CGM.CreateRuntimeFunction(FT, Name, {},
-                                                     /*Local=*/false,
-                                                     /*AssumeConvergent=*/true),
+    return EmitRuntimeCall(Intrinsic::getOrInsertDeclaration(
+                               &CGM.getModule(), IID, {OpExpr->getType()}),
                            ArrayRef{OpExpr}, "hlsl.wave.active.sum");
   }
   case Builtin::BI__builtin_hlsl_wave_active_max: {
     // Due to the use of variadic arguments, explicitly retreive argument
     Value *OpExpr = EmitScalarExpr(E->getArg(0));
-    llvm::FunctionType *FT = llvm::FunctionType::get(
-        OpExpr->getType(), ArrayRef{OpExpr->getType()}, false);
     Intrinsic::ID IID = getWaveActiveMaxIntrinsic(
         getTarget().getTriple().getArch(), CGM.getHLSLRuntime(),
         E->getArg(0)->getType());
 
-    // Get overloaded name
-    std::string Name =
-        Intrinsic::getName(IID, ArrayRef{OpExpr->getType()}, &CGM.getModule());
-    return EmitRuntimeCall(CGM.CreateRuntimeFunction(FT, Name, {},
-                                                     /*Local=*/false,
-                                                     /*AssumeConvergent=*/true),
+    return EmitRuntimeCall(Intrinsic::getOrInsertDeclaration(
+                               &CGM.getModule(), IID, {OpExpr->getType()}),
                            ArrayRef{OpExpr}, "hlsl.wave.active.max");
   }
   case Builtin::BI__builtin_hlsl_wave_get_lane_index: {
diff --git a/clang/test/CodeGenHLSL/builtins/WaveActiveMax.hlsl b/clang/test/CodeGenHLSL/builtins/WaveActiveMax.hlsl
index 7891cfc1989af..be05a17cc3692 100644
--- a/clang/test/CodeGenHLSL/builtins/WaveActiveMax.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/WaveActiveMax.hlsl
@@ -16,7 +16,7 @@ int test_int(int expr) {
 }
 
 // CHECK-DXIL: declare [[TY]] @llvm.dx.wave.reduce.max.i32([[TY]]) #[[#attr:]]
-// CHECK-SPIRV: declare spir_func [[TY]] @llvm.spv.wave.reduce.max.i32([[TY]]) #[[#attr:]]
+// CHECK-SPIRV: declare [[TY]] @llvm.spv.wave.reduce.max.i32([[TY]]) #[[#attr:]]
 
 // CHECK-LABEL: test_uint64_t
 uint64_t test_uint64_t(uint64_t expr) {
@@ -27,7 +27,7 @@ uint64_t test_uint64_t(uint64_t expr) {
 }
 
 // CHECK-DXIL: declare [[TY]] @llvm.dx.wave.reduce.umax.i64([[TY]]) #[[#attr:]]
-// CHECK-SPIRV: declare spir_func [[TY]] @llvm.spv.wave.reduce.umax.i64([[TY]]) #[[#attr:]]
+// CHECK-SPIRV: declare [[TY]] @llvm.spv.wave.reduce.umax.i64([[TY]]) #[[#attr:]]
 
 // Test basic lowering to runtime function call with array and float value.
 
@@ -40,7 +40,7 @@ float4 test_floatv4(float4 expr) {
 }
 
 // CHECK-DXIL: declare [[TY1]] @llvm.dx.wave.reduce.max.v4f32([[TY1]]) #[[#attr]]
-// CHECK-SPIRV: declare spir_func [[TY1]] @llvm.spv.wave.reduce.max.v4f32([[TY1]]) #[[#attr]]
+// CHECK-SPIRV: declare [[TY1]] @llvm.spv.wave.reduce.max.v4f32([[TY1]]) #[[#attr]]
 
 // CHECK: attributes #[[#attr]] = {{{.*}} convergent {{.*}}}
 
diff --git a/clang/test/CodeGenHLSL/builtins/WaveActiveSum.hlsl b/clang/test/CodeGenHLSL/builtins/WaveActiveSum.hlsl
index 4bf423ccc1b82..1fc93c62c8db0 100644
--- a/clang/test/CodeGenHLSL/builtins/WaveActiveSum.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/WaveActiveSum.hlsl
@@ -16,7 +16,7 @@ int test_int(int expr) {
 }
 
 // CHECK-DXIL: declare [[TY]] @llvm.dx.wave.reduce.sum.i32([[TY]]) #[[#attr:]]
-// CHECK-SPIRV: declare spir_func [[TY]] @llvm.spv.wave.reduce.sum.i32([[TY]]) #[[#attr:]]
+// CHECK-SPIRV: declare [[TY]] @llvm.spv.wave.reduce.sum.i32([[TY]]) #[[#attr:]]
 
 // CHECK-LABEL: test_uint64_t
 uint64_t test_uint64_t(uint64_t expr) {
@@ -27,7 +27,7 @@ uint64_t test_uint64_t(uint64_t expr) {
 }
 
 // CHECK-DXIL: declare [[TY]] @llvm.dx.wave.reduce.usum.i64([[TY]]) #[[#attr:]]
-// CHECK-SPIRV: declare spir_func [[TY]] @llvm.spv.wave.reduce.sum.i64([[TY]]) #[[#attr:]]
+// CHECK-SPIRV: declare [[TY]] @llvm.spv.wave.reduce.sum.i64([[TY]]) #[[#attr:]]
 
 // Test basic lowering to runtime function call with array and float value.
 
@@ -40,6 +40,6 @@ float4 test_floatv4(float4 expr) {
 }
 
 // CHECK-DXIL: declare [[TY1]] @llvm.dx.wave.reduce.sum.v4f32([[TY1]]) #[[#attr]]
-// CHECK-SPIRV: declare spir_func [[TY1]] @llvm.spv.wave.reduce.sum.v4f32([[TY1]]) #[[#attr]]
+// CHECK-SPIRV: declare [[TY1]] @llvm.spv.wave.reduce.sum.v4f32([[TY1]]) #[[#attr]]
 
 // CHECK: attributes #[[#attr]] = {{{.*}} convergent {{.*}}}

HLSL uses CreateRuntimeFunction for two intrinsics. This is pretty
weird thing to do, and doesn't match what the rest of the file
does.

I suspect this might be because these are convergent calls, but
the intrinsics themselves are already marked convergent, so it's
not necessary for clang to manually add the attribute.
return EmitRuntimeCall(CGM.CreateRuntimeFunction(FT, Name, {},
/*Local=*/false,
/*AssumeConvergent=*/true),
return EmitRuntimeCall(Intrinsic::getOrInsertDeclaration(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct. @Keenuts did EmitRuntimeCall(Intrinsic::getOrInsertDeclaration( in https://github.com/llvm/llvm-project/pull/143127/files Thats what we expect to use for these convergence intrinsics.

Looks like this was just an accident introduced in this pr: https://github.com/llvm/llvm-project/pull/118580/files#diff-202c36399fe94363f22692e534129341972137a24721c385b1f1f05fb239dd79R19523 and then the patttern may have been copied by others.

Copy link
Member

@farzonl farzonl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikic nikic merged commit 1128a4f into llvm:main Jun 23, 2025
7 of 8 checks passed
@nikic nikic deleted the cghlsl-intrin branch June 23, 2025 15:37
miguelcsx pushed a commit to miguelcsx/llvm-project that referenced this pull request Jun 23, 2025
HLSL uses CreateRuntimeFunction for three intrinsics. This is pretty
unusual thing to do, and doesn't match what the rest of the file does.

I suspect this might be because these are convergent calls, but the
intrinsics themselves are already marked convergent, so it's not
necessary for clang to manually add the attribute.

This does lose the spir_func CC on the intrinsic declaration, but again,
CC should not be relevant to intrinsics at all.
Jaddyen pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 23, 2025
HLSL uses CreateRuntimeFunction for three intrinsics. This is pretty
unusual thing to do, and doesn't match what the rest of the file does.

I suspect this might be because these are convergent calls, but the
intrinsics themselves are already marked convergent, so it's not
necessary for clang to manually add the attribute.

This does lose the spir_func CC on the intrinsic declaration, but again,
CC should not be relevant to intrinsics at all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants