-
Notifications
You must be signed in to change notification settings - Fork 86
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
wgsl: Add AbstractFloat mix
execution tests
#3087
Conversation
Previews, as seen when this build job started (de68a79): |
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.
Generally looks good to me, with some typo
src/unittests/floating_point.spec.ts
Outdated
{ input: [-1.0, 1.0, 0.9], expected: 0.8 }, | ||
|
||
// Showing how precise and imprecise versions diff | ||
// Note that this expectation is 0 in f64 as 10.0 is much smaller that f63.negative.min, |
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.
// Note that this expectation is 0 in f64 as 10.0 is much smaller that f63.negative.min, | |
// Note that this expectation is 0 in f64 as 10.0 is much smaller that f64.negative.min, |
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.
Done
t.params, | ||
cases | ||
); | ||
}); | ||
g.test('abstract_float_nonmatching_vec2') |
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.
nit: Add an empty line here
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.
Done
.unimplemented(); | ||
.params(u => u.combine('inputSource', onlyConstInputSource)) | ||
.fn(async t => { | ||
const cases = await d.get('abstract_vec3_scalar'); |
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.
const cases = await d.get('abstract_vec3_scalar'); | |
const cases = await d.get('abstract_vec4_scalar'); |
And also the types below
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.
Done
FYI, I am going to delay landing this for a while, since Dawn is currently working on some CTS running issues, and I think this would just be more fuel on that fire. |
src/unittests/floating_point.spec.ts
Outdated
@@ -5342,10 +5342,9 @@ const kMixImpreciseIntervalCases = { | |||
{ input: [-1.0, 1.0, 0.9], expected: [reinterpretU64AsF64(0x3fe9_9999_8000_0000n), reinterpretU64AsF64(0x3fe9_9999_c000_0000n)] }, // ~0.8 | |||
|
|||
// Showing how precise and imprecise versions diff | |||
// Note that this expectation is 0 only in f32 as 10.0 is much smaller that f32.negative.min, | |||
// Note that this expectation is 0 in f32 as 10.0 is much smaller that f32.negative.min, | |||
// So that 10 - f32.negative.min == f32.negative.min even in f64. But for f16, there is not |
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.
10 - f32.negative.min
is -f32.negative.min
, no?
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.
Done
src/unittests/floating_point.spec.ts
Outdated
// So that 10 - f32.negative.min == f32.negative.min even in f64. But for f16, there is not | ||
// a exactly-represenatble f16 value v that make v - f16.negative.min == f16.negative.min | ||
// in f64, in fact that require v being smaller than 2**-37. | ||
// an exactly-represenatble f16 value v that make v - f16.negative.min == f16.negative.min |
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.
Ditto
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.
Done
src/unittests/floating_point.spec.ts
Outdated
// So that 10 - f32.negative.min == f32.negative.min even in f64. But for f16, there is not | ||
// a exactly-represenatble f16 value v that make v - f16.negative.min == f16.negative.min | ||
// in f64, in fact that require v being smaller than 2**-37. | ||
// an exactly-represenatble f16 value v that make v - f16.negative.min == f16.negative.min | ||
{ input: [kValue.f32.negative.min, 10.0, 1.0], expected: 0.0 }, |
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.
Would be nice to see some cases with 1.0
changed to something more than and less than 1.0
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.
Done
de68a79
to
f344018
Compare
5249a6d
to
7fa5be9
Compare
7fa5be9
to
9921301
Compare
Fixes #2566
Requirements for PR author:
.unimplemented()
./** documented */
and new helper files are found inhelper_index.txt
.Requirements for reviewer sign-off:
When landing this PR, be sure to make any necessary issue status updates.