[HLSL] prevent generation of wrong double intrinsics. #86932

ritskii · 2024-03-28T10:53:41Z

As #86555, we should cover all of non-double builtins.

Closes #86818

…ying ceil, floor double type tests.

llvmbot · 2024-03-28T10:54:11Z

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Andrii Levitskiy (aabysswalker)

Changes

As #86555, we should cover all of non-double builtins.

Closes #86818

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

5 Files Affected:

(modified) clang/lib/Headers/hlsl/hlsl_intrinsics.h (-18)
(modified) clang/lib/Sema/SemaChecking.cpp (+5-1)
(modified) clang/test/CodeGenHLSL/builtins/ceil.hlsl (-13)
(modified) clang/test/CodeGenHLSL/builtins/floor.hlsl (-13)
(modified) clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl (+4-3)

diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index d47eab453f8747..82e2cff2cb95c5 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -243,15 +243,6 @@ float3 ceil(float3);
 _HLSL_BUILTIN_ALIAS(__builtin_elementwise_ceil)
 float4 ceil(float4);
 
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_ceil)
-double ceil(double);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_ceil)
-double2 ceil(double2);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_ceil)
-double3 ceil(double3);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_ceil)
-double4 ceil(double4);
-
 //===----------------------------------------------------------------------===//
 // clamp builtins
 //===----------------------------------------------------------------------===//
@@ -585,15 +576,6 @@ float3 floor(float3);
 _HLSL_BUILTIN_ALIAS(__builtin_elementwise_floor)
 float4 floor(float4);
 
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_floor)
-double floor(double);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_floor)
-double2 floor(double2);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_floor)
-double3 floor(double3);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_floor)
-double4 floor(double4);
-
 //===----------------------------------------------------------------------===//
 // frac builtins
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index f5c0c761da75af..2e4e18a3ebf759 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5641,12 +5641,16 @@ bool Sema::CheckHLSLBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
       return true;
     break;
   }
+  case Builtin::BI__builtin_elementwise_ceil:
   case Builtin::BI__builtin_elementwise_cos:
-  case Builtin::BI__builtin_elementwise_sin:
+  case Builtin::BI__builtin_elementwise_exp:
+  case Builtin::BI__builtin_elementwise_exp2:
+  case Builtin::BI__builtin_elementwise_floor:
   case Builtin::BI__builtin_elementwise_log:
   case Builtin::BI__builtin_elementwise_log2:
   case Builtin::BI__builtin_elementwise_log10:
   case Builtin::BI__builtin_elementwise_pow:
+  case Builtin::BI__builtin_elementwise_sin:
   case Builtin::BI__builtin_elementwise_sqrt:
   case Builtin::BI__builtin_elementwise_trunc: {
     if (CheckFloatOrHalfRepresentations(this, TheCall))
diff --git a/clang/test/CodeGenHLSL/builtins/ceil.hlsl b/clang/test/CodeGenHLSL/builtins/ceil.hlsl
index 06d0d4c2cf546d..be7725cd4d66c1 100644
--- a/clang/test/CodeGenHLSL/builtins/ceil.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/ceil.hlsl
@@ -41,16 +41,3 @@ float3 test_ceil_float3(float3 p0) { return ceil(p0); }
 // CHECK: define noundef <4 x float> @
 // CHECK: call <4 x float> @llvm.ceil.v4f32(
 float4 test_ceil_float4(float4 p0) { return ceil(p0); }
-
-// CHECK: define noundef double @
-// CHECK: call double @llvm.ceil.f64(
-double test_ceil_double(double p0) { return ceil(p0); }
-// CHECK: define noundef <2 x double> @
-// CHECK: call <2 x double> @llvm.ceil.v2f64(
-double2 test_ceil_double2(double2 p0) { return ceil(p0); }
-// CHECK: define noundef <3 x double> @
-// CHECK: call <3 x double> @llvm.ceil.v3f64(
-double3 test_ceil_double3(double3 p0) { return ceil(p0); }
-// CHECK: define noundef <4 x double> @
-// CHECK: call <4 x double> @llvm.ceil.v4f64(
-double4 test_ceil_double4(double4 p0) { return ceil(p0); }
diff --git a/clang/test/CodeGenHLSL/builtins/floor.hlsl b/clang/test/CodeGenHLSL/builtins/floor.hlsl
index d2a2f6e52f1ec3..07803bfae3be68 100644
--- a/clang/test/CodeGenHLSL/builtins/floor.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/floor.hlsl
@@ -41,16 +41,3 @@ float3 test_floor_float3(float3 p0) { return floor(p0); }
 // CHECK: define noundef <4 x float> @
 // CHECK: call <4 x float> @llvm.floor.v4f32(
 float4 test_floor_float4(float4 p0) { return floor(p0); }
-
-// CHECK: define noundef double @
-// CHECK: call double @llvm.floor.f64(
-double test_floor_double(double p0) { return floor(p0); }
-// CHECK: define noundef <2 x double> @
-// CHECK: call <2 x double> @llvm.floor.v2f64(
-double2 test_floor_double2(double2 p0) { return floor(p0); }
-// CHECK: define noundef <3 x double> @
-// CHECK: call <3 x double> @llvm.floor.v3f64(
-double3 test_floor_double3(double3 p0) { return floor(p0); }
-// CHECK: define noundef <4 x double> @
-// CHECK: call <4 x double> @llvm.floor.v4f64(
-double4 test_floor_double4(double4 p0) { return floor(p0); }
diff --git a/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl
index 92dd06d5a33095..4ab36146ee4e69 100644
--- a/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl
@@ -1,12 +1,13 @@
-// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_cos 
-// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_sin 
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_ceil
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_cos
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_floor 
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_log 
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_log2 
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_log10
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_sin
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_sqrt
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_trunc
 
-
 double2 test_double_builtin(double2 p0) {
     return TEST_FUNC(p0);
   // expected-error@-1 {{passing 'double2' (aka 'vector<double, 2>') to parameter of incompatible type '__attribute__((__vector_size__(2 * sizeof(float)))) float' (vector of 2 'float' values)}}

farzonl · 2024-03-28T13:17:57Z

I was writing up a ticket for floor and ceil. Nice catch!
https://godbolt.org/z/q9a644e6W.

farzonl · 2024-03-28T13:18:51Z

clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl

 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_log 
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_log2 
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_log10
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_sin
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_sqrt
 // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -DTEST_FUNC=__builtin_elementwise_trunc



in semaChecking You added __builtin_elementwise_exp and __builtin_elementwise_exp2 you should add them to the tests

i though https://github.com/llvm/llvm-project/blob/main/clang/test/SemaHLSL/BuiltIns/exp-errors.hlsl#L1-L27
would cover that, but seems like it miss double type check, should i add test to half-float-only-errors.hlsl?

exp-errors.hlsl covers clangs behavior of exp elementwise builtins. The CheckHLSLBuiltinFunctionCall function you added exp \ exp2 is used to apply hlsl specific behaviors. Since this is a deviation from normal behvaior of the elementwise builtin it would be better to add the tests to half-float-only-errors.hlsl

There are a few expections to this rule half-float-only-errors.hlsl only covers unary functions. Since there weren't enough binary cases pow got its own file.

farzonl · 2024-03-28T13:21:46Z

So keep in mind there are some double cases that are valid for hlsl. so always check via dxc which can be easily used via hlsl.godbolt.

ritskii · 2024-03-28T14:40:25Z

thanks for review! can you merge that?

farzonl · 2024-03-28T14:41:46Z

thanks for review! can you merge that?

Our process is two reviewers. I will after one more person takes a look.

Prevent generating double of ceil, exp, exp2, floor. Removed accompan…

caec748

…ying ceil, floor double type tests.

llvmbot added clang Clang issues not falling into any other category backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics HLSL HLSL Language Support labels Mar 28, 2024

farzonl reviewed Mar 28, 2024

View reviewed changes

add exp test cases

3e05c22

ritskii changed the title ~~[HLSL] prevent generation of double intrinsics.~~ [HLSL] prevent generation of wrong double intrinsics. Mar 28, 2024

farzonl approved these changes Mar 28, 2024

View reviewed changes

python3kgae approved these changes Mar 28, 2024

View reviewed changes

farzonl merged commit 6dceea3 into llvm:main Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[HLSL] prevent generation of wrong double intrinsics. #86932

[HLSL] prevent generation of wrong double intrinsics. #86932

Uh oh!

ritskii commented Mar 28, 2024

Uh oh!

llvmbot commented Mar 28, 2024 •

edited

Loading

Uh oh!

farzonl commented Mar 28, 2024

Uh oh!

farzonl Mar 28, 2024

Uh oh!

ritskii Mar 28, 2024

Uh oh!

farzonl Mar 28, 2024

Uh oh!

farzonl Mar 28, 2024

Uh oh!

farzonl commented Mar 28, 2024

Uh oh!

ritskii commented Mar 28, 2024

Uh oh!

farzonl commented Mar 28, 2024

Uh oh!

Uh oh!

[HLSL] prevent generation of wrong double intrinsics. #86932

[HLSL] prevent generation of wrong double intrinsics. #86932

Uh oh!

Conversation

ritskii commented Mar 28, 2024

Uh oh!

llvmbot commented Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

farzonl commented Mar 28, 2024

Uh oh!

farzonl Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

ritskii Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

farzonl Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

farzonl Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

farzonl commented Mar 28, 2024

Uh oh!

ritskii commented Mar 28, 2024

Uh oh!

farzonl commented Mar 28, 2024

Uh oh!

Uh oh!

llvmbot commented Mar 28, 2024 •

edited

Loading