Skip to content

[HLSL] prevent generation of double intrinsics via the builtins #86555

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

Conversation

farzonl
Copy link
Member

@farzonl farzonl commented Mar 25, 2024

fixes #86551
closes #86552

Thanks to #86440 and #86407 it makes more sense for us to do type checks early via Sema to prevent the generation of invalid intrinsics.

@llvmbot 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 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 25, 2024

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-clang

Author: Farzon Lotfi (farzonl)

Changes

fixes #86551
closes #86552

Thanks to #86440 and #86407 it makes more sense for us to do type checks early via Sema to prevent the generation of invalid intrinsics.


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

4 Files Affected:

  • (modified) clang/lib/Headers/hlsl/hlsl_intrinsics.h (-9)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+14)
  • (added) clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl (+12)
  • (added) clang/test/SemaHLSL/BuiltIns/pow-errors.hlsl (+6)
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index d9f27c0db57ce4..0f9504de3b4992 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -392,15 +392,6 @@ float3 cos(float3);
 _HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
 float4 cos(float4);
 
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
-double cos(double);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
-double2 cos(double2);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
-double3 cos(double3);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
-double4 cos(double4);
-
 //===----------------------------------------------------------------------===//
 // dot product builtins
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 246e3577809a79..578c2aa26fd6b0 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5624,6 +5624,20 @@ bool Sema::CheckHLSLBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
             TheCall, /*CheckForFloatArgs*/
             TheCall->getArg(0)->getType()->hasFloatingRepresentation()))
       return true;
+    break;
+  }
+  // Note these are llvm builtins that we want to catch invalid intrinsic
+  // generation. Normal handling of these builitns will occur elsewhere.
+  case Builtin::BI__builtin_elementwise_cos:
+  case Builtin::BI__builtin_elementwise_sin:
+  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_trunc: {
+    if (CheckFloatOrHalfRepresentations(this, TheCall))
+      return true;
+    break;
   }
   }
   return false;
diff --git a/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl
new file mode 100644
index 00000000000000..66ab47df51e5e8
--- /dev/null
+++ b/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl
@@ -0,0 +1,12 @@
+// 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_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_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)}}
+}
diff --git a/clang/test/SemaHLSL/BuiltIns/pow-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/pow-errors.hlsl
new file mode 100644
index 00000000000000..949028aacf24b6
--- /dev/null
+++ b/clang/test/SemaHLSL/BuiltIns/pow-errors.hlsl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify
+
+double2 test_double_builtin(double2 p0, double2 p1) {
+    return __builtin_elementwise_pow(p0,p1);
+  // 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)}}
+}

@llvmbot
Copy link
Member

llvmbot commented Mar 25, 2024

@llvm/pr-subscribers-hlsl

Author: Farzon Lotfi (farzonl)

Changes

fixes #86551
closes #86552

Thanks to #86440 and #86407 it makes more sense for us to do type checks early via Sema to prevent the generation of invalid intrinsics.


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

4 Files Affected:

  • (modified) clang/lib/Headers/hlsl/hlsl_intrinsics.h (-9)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+14)
  • (added) clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl (+12)
  • (added) clang/test/SemaHLSL/BuiltIns/pow-errors.hlsl (+6)
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index d9f27c0db57ce4..0f9504de3b4992 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -392,15 +392,6 @@ float3 cos(float3);
 _HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
 float4 cos(float4);
 
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
-double cos(double);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
-double2 cos(double2);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
-double3 cos(double3);
-_HLSL_BUILTIN_ALIAS(__builtin_elementwise_cos)
-double4 cos(double4);
-
 //===----------------------------------------------------------------------===//
 // dot product builtins
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 246e3577809a79..578c2aa26fd6b0 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5624,6 +5624,20 @@ bool Sema::CheckHLSLBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
             TheCall, /*CheckForFloatArgs*/
             TheCall->getArg(0)->getType()->hasFloatingRepresentation()))
       return true;
+    break;
+  }
+  // Note these are llvm builtins that we want to catch invalid intrinsic
+  // generation. Normal handling of these builitns will occur elsewhere.
+  case Builtin::BI__builtin_elementwise_cos:
+  case Builtin::BI__builtin_elementwise_sin:
+  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_trunc: {
+    if (CheckFloatOrHalfRepresentations(this, TheCall))
+      return true;
+    break;
   }
   }
   return false;
diff --git a/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl
new file mode 100644
index 00000000000000..66ab47df51e5e8
--- /dev/null
+++ b/clang/test/SemaHLSL/BuiltIns/half-float-only-errors.hlsl
@@ -0,0 +1,12 @@
+// 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_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_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)}}
+}
diff --git a/clang/test/SemaHLSL/BuiltIns/pow-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/pow-errors.hlsl
new file mode 100644
index 00000000000000..949028aacf24b6
--- /dev/null
+++ b/clang/test/SemaHLSL/BuiltIns/pow-errors.hlsl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify
+
+double2 test_double_builtin(double2 p0, double2 p1) {
+    return __builtin_elementwise_pow(p0,p1);
+  // 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)}}
+}

Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link

✅ With the latest revision this PR passed the Python code formatter.

@farzonl farzonl force-pushed the hlsl-intrinsic-enforce-types branch from 8d2d339 to 7c75d35 Compare March 25, 2024 22:32
@farzonl farzonl merged commit 366592b into llvm:main Mar 26, 2024
@farzonl farzonl deleted the hlsl-intrinsic-enforce-types branch March 27, 2024 01:43
farzonl pushed a commit that referenced this pull request Mar 28, 2024
As #86555, we should cover all of non-double builtins.

Closes #86818
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics 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.

[HLSL] add sema type checks for builtins to prevent invalid intrinsic generation in code gen. [HLSL] cos doesn't have a double intrinsic
4 participants