We should't require inlining. It should be a step that can be selectively applied, or not, and have no affect on correctness. Different code generation backends may or may not wish to invoke inlining in certain cases.
Created attachment 349039 [details] Patch
Comment on attachment 349039 [details] Patch Can we add a test that enables inlining for a couple functions to make sure it doesn't break?
visitCallExpression(node) in Evaluator has a comment: // We evaluate inlined ASTs, so this can only be a native call. This seems to be false now. Also callFunction() still calls resolveInlinedFunction(), which seems either wrong or poorly named.
Why does resolveInlinedFunction() call _inlineFunction()?
Created attachment 349352 [details] Patch
*** Bug 189326 has been marked as a duplicate of this bug. ***
Comment on attachment 349352 [details] 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?
Created attachment 349357 [details] Patch
Comment on attachment 349357 [details] Patch Clearing flags on attachment: 349357 Committed r235879: <https://trac.webkit.org/changeset/235879>
All reviewed patches have been landed. Closing bug.
<rdar://problem/44326053>
Comment on attachment 349357 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349357&action=review > Tools/WebGPUShadingLanguageRI/LateCheckAndLayoutBuffers.js:36 > + for (let funcList of program.functions.values()) { > + for (let func of funcList) { > + if (func.isNative) > + continue; > + func.visit(new LateChecker()); > + func.visit(new EBufferBuilder()); > + } > + } Why do we have to alternate between LateChecker() and EBufferBuilder()? Why can't we separate these out into two separate calls in Prepare()?
Migrated to https://github.com/gpuweb/WHLSL/issues/98