-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[HLSL] Add bounds checks for the hlsl vector arguments and return types #130724
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i tried that, but if you do |
||
|
||
} // namespace __detail | ||
} // namespace hlsl | ||
#endif //_HLSL_HLSL_DETAILS_H_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 The annoying thing for this case was I needed to distinguish the float and half scalar templates to be able to apply There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) {...} |
||
__detail::is_same<half, T>::value, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah this was weird but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree it makes sense to leave as a cleanup, but we should use lower case |
||
T> 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, T> | ||
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, | ||
T> 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, T> | ||
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, | ||
T> 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, T> | ||
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 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HLSL_FIXED_VECTOR
looks like a macro to me rather than a type. Why use all caps convention here?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was using it like a macro for creating the vector type.