Add "if" statements to WSL
Created attachment 319763 [details] WIP
Comment on attachment 319763 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=319763&action=review > Tools/WebGPUShadingLanguageRI/NameResolver.js:178 > + let argumentIsType = node.typeArguments[i] instanceof Type || node.typeArguments[i] instanceof TypeOrVariableRef; I'm not sure about this. We may need to have an intermediary pass where we replace all the TypeOrVariableRefs with the right thing. WDYT, Fil?
Comment on attachment 319763 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=319763&action=review >> Tools/WebGPUShadingLanguageRI/NameResolver.js:178 >> + let argumentIsType = node.typeArguments[i] instanceof Type || node.typeArguments[i] instanceof TypeOrVariableRef; > > I'm not sure about this. We may need to have an intermediary pass where we replace all the TypeOrVariableRefs with the right thing. WDYT, Fil? It’s NameResolver’s job. Look for a typo in _resolveTypeArguments: typeArgument[i] should be typeArguments[i]. I fixed it in one of my patches.
Created attachment 320210 [details] Patch
Comment on attachment 320210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320210&action=review R=me but I wouldn’t remove the type checks. > Tools/WebGPUShadingLanguageRI/Checker.js:222 > + // make sure that the operand is a bool. I would still check though.
Committed r221776: <http://trac.webkit.org/changeset/221776>
Comment on attachment 320210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320210&action=review > Tools/WebGPUShadingLanguageRI/Checker.js:221 > + // The parser forcably emits a bool cast on the operand, so we don't need to Typo: should be forcibly > Tools/WebGPUShadingLanguageRI/Parse.js:583 > + if (token.text == "if") Nit: it feels like it’s time to make this a switch instead of cascading “if”s
Comment on attachment 320210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320210&action=review >> Tools/WebGPUShadingLanguageRI/Checker.js:221 >> + // The parser forcably emits a bool cast on the operand, so we don't need to > > Typo: should be forcibly Ignore me your landed patch doesn’t have this comment
Comment on attachment 320210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320210&action=review > Tools/WebGPUShadingLanguageRI/Evaluator.js:160 > + visitIfStatement(node) > + { > + if (node.conditional.visit(this).loadValue()) > + return node.body.visit(this); > + else if (node.elseBody) > + return node.elseBody.visit(this); > + } Is the implicit return value expected? If so, maybe make it explicit, I don't see any other code expecting/depending on implicit return values. if (...) return node.body.visit(this); if (node.elseBody) return node.elseBody.visit(this); return undefined;
(In reply to Myles C. Maxfield from comment #6) > Committed r221776: <http://trac.webkit.org/changeset/221776> This change caused wsl-tests.yaml/Test.js.ftl-eager-no-cjit to time out on debug JSC bots. Details in https://bugs.webkit.org/show_bug.cgi?id=176631
Reverted r221776 for reason: This change caused wsl-tests.yaml/Test.js to time out on Debug JSC bots. Committed r221824: <http://trac.webkit.org/changeset/221824>
(In reply to Ryan Haddad from comment #11) > Reverted r221776 for reason: > > This change caused wsl-tests.yaml/Test.js to time out on Debug JSC bots. > > Committed r221824: <http://trac.webkit.org/changeset/221824> In the future, can we skip this test instead, if it times out? Timeouts on this test are rarely real bugs, so rolling things out due to timeouts on this test doesn’t help us.
*** Bug 176631 has been marked as a duplicate of this bug. ***
Relanded in https://trac.webkit.org/changeset/221829/webkit
<rdar://problem/34693704>
Migrated to https://github.com/gpuweb/WHLSL/issues/156