[webkit-reviews] review granted: [Bug 188641] [WHLSL] Inlining should be optional : [Attachment 349352] Patch

[bugzilla-daemon at webkit.org]

Myles C. Maxfield <mmaxfield at apple.com> has granted Thomas Denney
<tdenney at apple.com>'s request for review:
Bug 188641: [WHLSL] Inlining should be optional
https://bugs.webkit.org/show_bug.cgi?id=188641

Attachment 349352: Patch

https://bugs.webkit.org/attachment.cgi?id=349352&action=review




--- Comment #7 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 349352
  --> https://bugs.webkit.org/attachment.cgi?id=349352
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349352&action=review

> Tools/WebGPUShadingLanguageRI/CallFunction.js:31
> +    let overload = program.globalNameContext.resolveFuncOverload(name,
argumentTypes, null, true);

Cool.

> Tools/WebGPUShadingLanguageRI/Evaluator.js:40
> +	   // This only occurs when a function returns void.

Comments should state "why," not "what." I recommend deleting this.

> Tools/WebGPUShadingLanguageRI/Inline.js:39
>      func.rewrite(new Inliner(program, func, visiting));

We should make sure we never inline entry points. Do we have a test for that?

> Tools/WebGPUShadingLanguageRI/Test.js:7646
> +tests.evaluationOrderForArguments = () => {

Can we add some tests that do some inlining?