[WHLSL] Improve test suite type safety
Created attachment 349399 [details] Patch
Comment on attachment 349399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349399&action=review > Tools/WebGPUShadingLanguageRI/Casts.js:45 > +function castToBool(value) > +{ > + return !!value; > +} Can we put this with the rest of the casts? > Tools/WebGPUShadingLanguageRI/Test.js:83 > - return TypedValue.box(program.intrinsics.int, value); > + return TypedValue.box(program.intrinsics.int, castToInt(value)); > } > > function makeUint(program, value) > { > - return TypedValue.box(program.intrinsics.uint, value); > + return TypedValue.box(program.intrinsics.uint, castToUint(value)); > } > > function makeUchar(program, value) > { > - return TypedValue.box(program.intrinsics.uchar, value); > + return TypedValue.box(program.intrinsics.uchar, castToUchar(value)); > } > > function makeBool(program, value) > { > - return TypedValue.box(program.intrinsics.bool, value); > + return TypedValue.box(program.intrinsics.bool, castToBool(value)); > } > > function makeFloat(program, value) > { > - return TypedValue.box(program.intrinsics.float, value); > + return TypedValue.box(program.intrinsics.float, castToFloat(value)); > } > > function makeHalf(program, value) > { > - return TypedValue.box(program.intrinsics.half, value); > + return TypedValue.box(program.intrinsics.half, castToHalf(value)); perhaps instead of silently casting, we should throw if the input thing isn't already the right type. The caller should know if the only reason the test is passing is that the type system is interfering > Tools/WebGPUShadingLanguageRI/Test.js:2702 > - checkUchar(program, callFunction(program, "foo", [makeUchar(program, 65535), makeUint(program, 2)]), 255); > - checkUchar(program, callFunction(program, "foo", [makeUchar(program, -1), makeUint(program, 5)]), 255); > + checkUchar(program, callFunction(program, "foo", [makeUchar(program, 65535), makeUint(program, 2)]), 63); > + checkUchar(program, callFunction(program, "foo", [makeUchar(program, -1), makeUint(program, 5)]), 7); What? Why is this necessary?
(In reply to Myles C. Maxfield from comment #2) > Comment on attachment 349399 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=349399&action=review > > > Tools/WebGPUShadingLanguageRI/Casts.js:45 > > +function castToBool(value) > > +{ > > + return !!value; > > +} > > Can we put this with the rest of the casts? It is! > > > Tools/WebGPUShadingLanguageRI/Test.js:83 > > - return TypedValue.box(program.intrinsics.int, value); > > + return TypedValue.box(program.intrinsics.int, castToInt(value)); > > } > > > > function makeUint(program, value) > > { > > - return TypedValue.box(program.intrinsics.uint, value); > > + return TypedValue.box(program.intrinsics.uint, castToUint(value)); > > } > > > > function makeUchar(program, value) > > { > > - return TypedValue.box(program.intrinsics.uchar, value); > > + return TypedValue.box(program.intrinsics.uchar, castToUchar(value)); > > } > > > > function makeBool(program, value) > > { > > - return TypedValue.box(program.intrinsics.bool, value); > > + return TypedValue.box(program.intrinsics.bool, castToBool(value)); > > } > > > > function makeFloat(program, value) > > { > > - return TypedValue.box(program.intrinsics.float, value); > > + return TypedValue.box(program.intrinsics.float, castToFloat(value)); > > } > > > > function makeHalf(program, value) > > { > > - return TypedValue.box(program.intrinsics.half, value); > > + return TypedValue.box(program.intrinsics.half, castToHalf(value)); > > perhaps instead of silently casting, we should throw if the input thing > isn't already the right type. The caller should know if the only reason the > test is passing is that the type system is interfering OK, I’ll add this now. > > > Tools/WebGPUShadingLanguageRI/Test.js:2702 > > - checkUchar(program, callFunction(program, "foo", [makeUchar(program, 65535), makeUint(program, 2)]), 255); > > - checkUchar(program, callFunction(program, "foo", [makeUchar(program, -1), makeUint(program, 5)]), 255); > > + checkUchar(program, callFunction(program, "foo", [makeUchar(program, 65535), makeUint(program, 2)]), 63); > > + checkUchar(program, callFunction(program, "foo", [makeUchar(program, -1), makeUint(program, 5)]), 7); > > What? Why is this necessary? makeUchar of -1 yields the value 0xFF (likewise with 65535), so right shifting these values by 2 and 5 yields 63 and 7 respectively.
Comment on attachment 349399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349399&action=review >>> Tools/WebGPUShadingLanguageRI/Casts.js:45 >>> +} >> >> Can we put this with the rest of the casts? > > It is! Nevermind, you're right. >>> Tools/WebGPUShadingLanguageRI/Test.js:2702 >>> + checkUchar(program, callFunction(program, "foo", [makeUchar(program, -1), makeUint(program, 5)]), 7); >> >> What? Why is this necessary? > > makeUchar of -1 yields the value 0xFF (likewise with 65535), so right shifting these values by 2 and 5 yields 63 and 7 respectively. I see, the test was testing incorrect behavior before. Either way, the test shouldn't say makeUchar(program, -1). It should say makeUchar(program, 255). Same with 65535.
Created attachment 349429 [details] Patch
Comment on attachment 349429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349429&action=review > Tools/WebGPUShadingLanguageRI/All.js:105 > +load("IsBitwiseEquivalent.js"); Why not put this in Casts.js? > Tools/WebGPUShadingLanguageRI/Casts.js:126 > + throw new Error(`${value} was casted to ${castedValue}`); This message is not particularly clear why it is an error condition.
Created attachment 349456 [details] Patch
Comment on attachment 349456 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349456&action=review > Tools/WebGPUShadingLanguageRI/Test.js:-2625 > - checkUint(program, callFunction(program, "foo", [makeUint(program, -1), makeUint(program, 5)]), 134217727); Can we replace all of these tests with 2^32-1 instead?
Created attachment 350126 [details] Patch
Comment on attachment 350126 [details] Patch Clearing flags on attachment: 350126 Committed r236217: <https://trac.webkit.org/changeset/236217>
All reviewed patches have been landed. Closing bug.
<rdar://problem/44613690>