Skip to content

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

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.

Sign up for GitHub

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 2 commits into from
Mar 28, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
caec748
Prevent generating double of ceil, exp, exp2, floor. Removed accompan…
ritskii Mar 28, 2024
3e05c22
add exp test cases
ritskii Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions clang/lib/Headers/hlsl/hlsl_intrinsics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -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
//===----------------------------------------------------------------------===//
Expand Down
6 changes: 5 additions & 1 deletion clang/lib/Sema/SemaChecking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
13 changes: 0 additions & 13 deletions clang/test/CodeGenHLSL/builtins/ceil.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
13 changes: 0 additions & 13 deletions clang/test/CodeGenHLSL/builtins/floor.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
9 changes: 6 additions & 3 deletions clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// 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_exp
// 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_exp2
// 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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.


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)}}
Expand Down