[HLSL] Add bounds checks for the hlsl vector arguments and return types #130724
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-backend-x86 Author: Farzon Lotfi (farzonl) Changesfixes #129003
Patch is 24.37 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130724.diff 6 Files Affected:
diff --git a/clang/lib/Headers/hlsl/hlsl_detail.h b/clang/lib/Headers/hlsl/hlsl_detail.h
index c691d85283de4..80c4900121dfb 100644
--- a/clang/lib/Headers/hlsl/hlsl_detail.h
+++ b/clang/lib/Headers/hlsl/hlsl_detail.h
@@ -45,6 +45,10 @@ template <typename T> struct is_arithmetic {
static const bool Value = __is_arithmetic(T);
};
+template <typename T, int N>
+using HLSL_FIXED_VECTOR =
+ vector<__detail::enable_if_t<(N > 1 && N <= 4), T>, N>;
+
} // namespace __detail
} // namespace hlsl
#endif //_HLSL_HLSL_DETAILS_H_
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
index 6783e23f6346d..87b52792447f6 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
@@ -23,11 +23,7 @@ constexpr vector<uint, 4> d3d_color_to_ubyte4_impl(vector<float, 4> V) {
return V.zyxw * 255.001953f;
}
-template <typename T>
-constexpr enable_if_t<is_same<float, T>::value || is_same<half, T>::value, T>
-length_impl(T X) {
- return abs(X);
-}
+template <typename T> constexpr T length_impl(T X) { return abs(X); }
template <typename T, int N>
constexpr enable_if_t<is_same<float, T>::value || is_same<half, T>::value, T>
@@ -39,9 +35,7 @@ length_vec_impl(vector<T, N> X) {
#endif
}
-template <typename T>
-constexpr enable_if_t<is_same<float, T>::value || is_same<half, T>::value, T>
-distance_impl(T X, T Y) {
+template <typename T> constexpr T distance_impl(T X, T Y) {
return length_impl(X - Y);
}
@@ -51,9 +45,7 @@ distance_vec_impl(vector<T, N> X, vector<T, N> Y) {
return length_vec_impl(X - Y);
}
-template <typename T>
-constexpr enable_if_t<is_same<float, T>::value || is_same<half, T>::value, T>
-reflect_impl(T I, T N) {
+template <typename T> constexpr T reflect_impl(T I, T N) {
return I - 2 * N * I * N;
}
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index cd6d836578787..3fadd45ecabb7 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -89,23 +89,31 @@ void asuint(double4, out uint4, out uint4);
/// \param X The X input value.
/// \param Y The Y input value.
+template <typename T>
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
-const inline half distance(half X, half Y) {
+const inline __detail::enable_if_t<
+ __detail::is_arithmetic<T>::Value &&
+ __detail::is_same<half, T>::value> distance(T X, T Y) {
return __detail::distance_impl(X, Y);
}
-const inline float distance(float X, float Y) {
+template <typename T>
+const inline __detail::enable_if_t<__detail::is_arithmetic<T>::Value &&
+ __detail::is_same<float, T>::value>
+distance(T X, T Y) {
return __detail::distance_impl(X, Y);
}
template <int N>
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
-const inline half distance(vector<half, N> X, vector<half, N> Y) {
+const inline half distance(__detail::HLSL_FIXED_VECTOR<half, N> X,
+ __detail::HLSL_FIXED_VECTOR<half, N> Y) {
return __detail::distance_vec_impl(X, Y);
}
template <int N>
-const inline float distance(vector<float, N> X, vector<float, N> Y) {
+const inline float distance(__detail::HLSL_FIXED_VECTOR<float, N> X,
+ __detail::HLSL_FIXED_VECTOR<float, N> Y) {
return __detail::distance_vec_impl(X, Y);
}
@@ -119,17 +127,29 @@ const inline float distance(vector<float, N> X, vector<float, N> Y) {
///
/// Length is based on the following formula: sqrt(x[0]^2 + x[1]^2 + ...).
+template <typename T>
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
-const inline half length(half X) { return __detail::length_impl(X); }
-const inline float length(float X) { return __detail::length_impl(X); }
+const inline __detail::enable_if_t<
+ __detail::is_arithmetic<T>::Value &&
+ __detail::is_same<half, T>::value> length(T X) {
+ return __detail::length_impl(X);
+}
+
+template <typename T>
+const inline __detail::enable_if_t<__detail::is_arithmetic<T>::Value &&
+ __detail::is_same<float, T>::value>
+length(T X) {
+ return __detail::length_impl(X);
+}
template <int N>
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
-const inline half length(vector<half, N> X) {
+const inline half length(__detail::HLSL_FIXED_VECTOR<half, N> X) {
return __detail::length_vec_impl(X);
}
-template <int N> const inline float length(vector<float, N> X) {
+template <int N>
+const inline float length(__detail::HLSL_FIXED_VECTOR<float, N> X) {
return __detail::length_vec_impl(X);
}
@@ -173,23 +193,33 @@ constexpr vector<uint, 4> D3DCOLORtoUBYTE4(vector<float, 4> V) {
///
/// Result type and the type of all operands must be the same type.
+template <typename T>
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
-const inline half reflect(half I, half N) {
+const inline __detail::enable_if_t<
+ __detail::is_arithmetic<T>::Value &&
+ __detail::is_same<half, T>::value> reflect(T I, T N) {
return __detail::reflect_impl(I, N);
}
-const inline float reflect(float I, float N) {
+template <typename T>
+const inline __detail::enable_if_t<__detail::is_arithmetic<T>::Value &&
+ __detail::is_same<float, T>::value>
+reflect(T I, T N) {
return __detail::reflect_impl(I, N);
}
template <int L>
_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
-const inline vector<half, L> reflect(vector<half, L> I, vector<half, L> N) {
+const inline __detail::HLSL_FIXED_VECTOR<half, L> reflect(
+ __detail::HLSL_FIXED_VECTOR<half, L> I,
+ __detail::HLSL_FIXED_VECTOR<half, L> N) {
return __detail::reflect_vec_impl(I, N);
}
template <int L>
-const inline vector<float, L> reflect(vector<float, L> I, vector<float, L> N) {
+const inline __detail::HLSL_FIXED_VECTOR<float, L>
+reflect(__detail::HLSL_FIXED_VECTOR<float, L> I,
+ __detail::HLSL_FIXED_VECTOR<float, L> N) {
return __detail::reflect_vec_impl(I, N);
}
} // namespace hlsl
diff --git a/clang/test/SemaHLSL/BuiltIns/distance-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/distance-errors.hlsl
index e996bf5d2cb7c..0ddde79c5ef10 100644
--- a/clang/test/SemaHLSL/BuiltIns/distance-errors.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/distance-errors.hlsl
@@ -3,8 +3,8 @@
float test_no_second_arg(float2 p0) {
return distance(p0);
// expected-error@-1 {{no matching function for call to 'distance'}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires 2 arguments, but 1 was provided}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires 2 arguments, but 1 was provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 1 was provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 1 was provided}}
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 1 was provided}}
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 1 was provided}}
}
@@ -12,22 +12,46 @@ float test_no_second_arg(float2 p0) {
float test_too_many_arg(float2 p0) {
return distance(p0, p0, p0);
// expected-error@-1 {{no matching function for call to 'distance'}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires 2 arguments, but 3 were provided}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires 2 arguments, but 3 were provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 3 were provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 3 were provided}}
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 3 were provided}}
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 3 were provided}}
}
float test_double_inputs(double p0, double p1) {
return distance(p0, p1);
- // expected-error@-1 {{call to 'distance' is ambiguous}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
+ // expected-error@-1 {{no matching function for call to 'distance'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
}
float test_int_inputs(int p0, int p1) {
return distance(p0, p1);
- // expected-error@-1 {{call to 'distance' is ambiguous}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
+ // expected-error@-1 {{no matching function for call to 'distance'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+}
+
+float1 test_vec1_inputs(float1 p0, float1 p1) {
+ return distance(p0, p1);
+ // expected-error@-1 {{no matching function for call to 'distance'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with N = 1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, half>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with N = 1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, float>'}}
+}
+
+typedef float float5 __attribute__((ext_vector_type(5)));
+
+float5 test_vec5_inputs(float5 p0, float5 p1) {
+ return distance(p0, p1);
+ // expected-error@-1 {{no matching function for call to 'distance'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with N = 5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, half>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with N = 5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, float>'}}
}
diff --git a/clang/test/SemaHLSL/BuiltIns/length-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/length-errors.hlsl
index a191f0419fbba..4ec3c7f3fee44 100644
--- a/clang/test/SemaHLSL/BuiltIns/length-errors.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/length-errors.hlsl
@@ -4,8 +4,8 @@ void test_too_few_arg()
{
return length();
// expected-error@-1 {{no matching function for call to 'length'}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires single argument 'X', but no arguments were provided}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires single argument 'X', but no arguments were provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'X', but no arguments were provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'X', but no arguments were provided}}
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'X', but no arguments were provided}}
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'X', but no arguments were provided}}
}
@@ -14,40 +14,68 @@ void test_too_many_arg(float2 p0)
{
return length(p0, p0);
// expected-error@-1 {{no matching function for call to 'length'}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires single argument 'X', but 2 arguments were provided}}
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'X', but 2 arguments were provided}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires single argument 'X', but 2 arguments were provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'X', but 2 arguments were provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'X', but 2 arguments were provided}}
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'X', but 2 arguments were provided}}
}
float double_to_float_type(double p0) {
return length(p0);
- // expected-error@-1 {{call to 'length' is ambiguous}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
+ // expected-error@-1 {{no matching function for call to 'length'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = double]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = double]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: could not match '__detail::HLSL_FIXED_VECTOR<half, N>' (aka 'vector<__detail::enable_if_t<(N > 1 && N <= 4), half>, N>') against 'double'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: could not match '__detail::HLSL_FIXED_VECTOR<float, N>' (aka 'vector<__detail::enable_if_t<(N > 1 && N <= 4), float>, N>') against 'double'}}
}
float bool_to_float_type_promotion(bool p1)
{
return length(p1);
- // expected-error@-1 {{call to 'length' is ambiguous}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
+ // expected-error@-1 {{no matching function for call to 'length'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = bool]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = bool]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: could not match '__detail::HLSL_FIXED_VECTOR<half, N>' (aka 'vector<__detail::enable_if_t<(N > 1 && N <= 4), half>, N>') against 'bool'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: could not match '__detail::HLSL_FIXED_VECTOR<float, N>' (aka 'vector<__detail::enable_if_t<(N > 1 && N <= 4), float>, N>') against 'bool'}}
}
float length_int_to_float_promotion(int p1)
{
return length(p1);
- // expected-error@-1 {{call to 'length' is ambiguous}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
+ // expected-error@-1 {{no matching function for call to 'length'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = int]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = int]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: could not match '__detail::HLSL_FIXED_VECTOR<half, N>' (aka 'vector<__detail::enable_if_t<(N > 1 && N <= 4), half>, N>') against 'int'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: could not match '__detail::HLSL_FIXED_VECTOR<float, N>' (aka 'vector<__detail::enable_if_t<(N > 1 && N <= 4), float>, N>') against 'int'}}
}
float2 length_int2_to_float2_promotion(int2 p1)
{
return length(p1);
- // expected-error@-1 {{call to 'length' is ambiguous}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function}}
+ // expected-error@-1 {{no matching function for call to 'length'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = int2]}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = int2]}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{1st parameter does not match adjusted type 'vector<int, [...]>' of argument [with N = 2]}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{1st parameter does not match adjusted type 'vector<int, [...]>' of argument [with N = 2]}}
+}
+
+float1 test_vec1_inputs(float1 p0) {
+ return length(p0);
+ // expected-error@-1 {{no matching function for call to 'length'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with N = 1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, half>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with N = 1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, float>'}}
+}
+
+typedef float float5 __attribute__((ext_vector_type(5)));
+
+float5 test_vec5_inputs(float5 p0) {
+ return length(p0);
+ // expected-error@-1 {{no matching function for call to 'length'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, void>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with N = 5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, half>'}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with N = 5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, float>'}}
}
diff --git a/clang/test/SemaHLSL/BuiltIns/reflect-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/reflect-errors.hlsl
index 28cf992ed602b..4535991a859aa 100644
--- a/clang/test/SemaHLSL/BuiltIns/reflect-errors.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/reflect-errors.hlsl
@@ -3,8 +3,8 @@
float test_no_second_arg(float2 p0) {
return reflect(p0);
// expected-error@-1 {{no matching function for call to 'reflect'}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires 2 arguments, but 1 was provided}}
- // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires 2 arguments, but 1 was provided}}
+ // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires 2 arguments, but 1 was pr...
[truncated]
|
fixes llvm#129003 - fix up sema tests - fix up templates for scalar and vector HLSL intrinsic overloads
farzonl
force-pushed
the
bugfix/issue-129003
branch
from
80f6687 to
17386ba
Compare
spall
reviewed
Mar 11, 2025
There was a problem hiding this comment.
Looks mostly good to me, a couple of questions though which applied to most of the templates in the code, but I only commented on the first instances I saw.
- Do you need to check T is arithmetic if you're also checking its half? Wont' the half check suffice?
- capitalize value here? I assume this didn't cause you an error but nice to be consistent with use of Caps
| @@ -45,6 +45,10 @@ template <typename T> struct is_arithmetic { | |||
| static const bool Value = __is_arithmetic(T); | |||
| }; | |||
|
|
|||
| template <typename T, int N> | |||
| using HLSL_FIXED_VECTOR = | |||
| vector<__detail::enable_if_t<(N > 1 && N <= 4), T>, N>; | |||
There was a problem hiding this comment.
should the enable_if_t not go around the entire vector<T,N>, instead of within the T part?
There was a problem hiding this comment.
i tried that, but if you do __detail::enable_if_t<(N > 1 && N <= 4),vector<T,N>>; the N template argument gets disambiguated. note: candidate template ignored: couldn't infer template argument 'N' https://godbolt.org/z/8a1sKWrcW
| _HLSL_16BIT_AVAILABILITY(shadermodel, 6.2) | ||
| const inline half distance(half X, half Y) { | ||
| const inline __detail::enable_if_t<__detail::is_arithmetic<T>::Value && | ||
| __detail::is_same<half, T>::value, |
There was a problem hiding this comment.
capitalize value here? I assume this didn't cause you an error but nice to be consistent with use of Caps
There was a problem hiding this comment.
yeah this was weird but is_same was lowercase value. didn't change it because thought it was out of scope of this change, but might do a cleanup pr after.
There was a problem hiding this comment.
Why is it out of the scope in this change if you added it? Sorry I don't understand.
There was a problem hiding this comment.
I didn't add is_same thats been in clang/lib/Headers/hlsl/hlsl_detail.h for a long time. predating most the other stuff like enable_if_t and is_arithmetic.
There was a problem hiding this comment.
Agree it makes sense to leave as a cleanup, but we should use lower case value here rather than llvm conventions since these helpers match the standard C++ versions of the same.
| _HLSL_16BIT_AVAILABILITY(shadermodel, 6.2) | ||
| const inline half distance(half X, half Y) { | ||
| const inline __detail::enable_if_t<__detail::is_arithmetic<T>::Value && |
There was a problem hiding this comment.
Do you need to check T is arithmetic if you're also checking its half? Wont' the half check suffice?
There was a problem hiding this comment.
Yeah I didn't want to templatize the scalar case, but what I found out was if I didn't then the vectors of any size would cast themselves down to scalars. The is_arithmetic here is to check the type coming in is already a scalar.
The annoying thing for this case was I needed to distinguish the float and half scalar templates to be able to apply _HLSL_16BIT_AVAILABILITY(shadermodel, 6.2) and thats why the is_same stuff is here.
There was a problem hiding this comment.
The other option would be to do a template that matches the non-fixed vectors and explicitly delete those overloads. Not sure which approach ends up being cleaner though.
There was a problem hiding this comment.
I didn't want to do that. Since 6.9 is going to add "long" vectors as part of cooperative vectors spec I figured we needed to do something like this long term
template <int N>
_HLSL_AVAILABILITY(shadermodel, 6.9)
const inline float distance(__detail::HLSL_LONG_VECTOR<float, N> X,
__detail::HLSL_LONG_VECTOR<float, N> Y) {...}
spall
approved these changes
Mar 11, 2025
bogner
approved these changes
Mar 11, 2025
| @@ -45,6 +45,10 @@ template <typename T> struct is_arithmetic { | |||
| static const bool Value = __is_arithmetic(T); | |||
| }; | |||
|
|
|||
| template <typename T, int N> | |||
| using HLSL_FIXED_VECTOR = | |||
There was a problem hiding this comment.
HLSL_FIXED_VECTOR looks like a macro to me rather than a type. Why use all caps convention here?
There was a problem hiding this comment.
I was using it like a macro for creating the vector type.
| _HLSL_16BIT_AVAILABILITY(shadermodel, 6.2) | ||
| const inline half distance(half X, half Y) { | ||
| const inline __detail::enable_if_t<__detail::is_arithmetic<T>::Value && |
There was a problem hiding this comment.
The other option would be to do a template that matches the non-fixed vectors and explicitly delete those overloads. Not sure which approach ends up being cleaner though.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #129003