From 2c778e587793d0db5b2a87eb26f65ea1d99a793d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 13 Dec 2024 13:14:59 -0800 Subject: [PATCH 1/8] fix --- tools/acorn-optimizer.mjs | 41 +++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index 18e99fd93cbc4..e96a25cbe5f2a 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -65,9 +65,13 @@ function simpleWalk(node, cs) { } } -// Full post-order walk, calling a single function for all types. -function fullWalk(node, c) { - visitChildren(node, (child) => fullWalk(child, c)); +// Full post-order walk, calling a single function for all types. If |pre| is +// provided, it is called in pre-order (before children). +function fullWalk(node, c, pre) { + if (pre) { + pre(node); + } + visitChildren(node, (child) => fullWalk(child, c, pre)); c(node); } @@ -699,6 +703,13 @@ function emitDCEGraph(ast) { } } + // We track defined functions very carefully, so that we can remove them and + // the things they call, but other function scopes (like arrow functions and + // object methods) are trickier to track (object methods require knowing what + // object a function name is called on), so we do not track those. We consider + // all content inside them as top-level, which means it is used. + var specialScopes = 0; + fullWalk(ast, (node) => { if (isWasmImportsAssign(node)) { const assignedObject = getWasmImportsValue(node); @@ -788,11 +799,14 @@ function emitDCEGraph(ast) { emptyOut(node); } } else if (node.type === 'FunctionDeclaration') { - defuns.push(node); - const name = node.id.name; - nameToGraphName[name] = getGraphName(name, 'defun'); - emptyOut(node); // ignore this in the second pass; we scan defuns separately + if (!specialScopes) { + defuns.push(node); + const name = node.id.name; + nameToGraphName[name] = getGraphName(name, 'defun'); + emptyOut(node); // ignore this in the second pass; we scan defuns separately + } } else if (node.type === 'ArrowFunctionExpression') { + specialScopes--; // Check if this is the minimal runtime exports function, which looks like // (output) => { var wasmExports = output.instance.exports; if ( @@ -857,9 +871,19 @@ function emitDCEGraph(ast) { } } } + } else if (node.type === 'Property' && node.method) { + specialScopes--; + } + }, (node) => { + // Pre-walking logic. We note special scopes (see above). + if (node.type === 'ArrowFunctionExpression' || + (node.type === 'Property' && node.method)) { + specialScopes++; } }); - // must find the info we need + // Scoping must balance out. + assert(specialScopes === 0); + // We must have found the info we need. assert( foundWasmImportsAssign, 'could not find the assignment to "wasmImports". perhaps --pre-js or --post-js code moved it out of the global scope? (things like that should be done after emcc runs, as they do not need to be run through the optimizer which is the special thing about --pre-js/--post-js code)', @@ -870,6 +894,7 @@ function emitDCEGraph(ast) { saveAsmExport(exp[0], exp[1]); } } + // Second pass: everything used in the toplevel scope is rooted; // things used in defun scopes create links function getGraphName(name, what) { From aa330fe6bed724bde12c638ce8dc8e23aa5fae92 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 13 Dec 2024 13:16:27 -0800 Subject: [PATCH 2/8] work --- test/js_optimizer/emitDCEGraph6-output.js | 24 ++++++++++++++++ test/js_optimizer/emitDCEGraph6.js | 35 +++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 test/js_optimizer/emitDCEGraph6-output.js create mode 100644 test/js_optimizer/emitDCEGraph6.js diff --git a/test/js_optimizer/emitDCEGraph6-output.js b/test/js_optimizer/emitDCEGraph6-output.js new file mode 100644 index 0000000000000..c70fac302fc31 --- /dev/null +++ b/test/js_optimizer/emitDCEGraph6-output.js @@ -0,0 +1,24 @@ +[ + { + "name": "emcc$defun$arrowed", + "reaches": [], + "root": true + }, + { + "name": "emcc$defun$bar", + "reaches": [], + "root": true + }, + { + "name": "emcc$defun$caller", + "reaches": [ + "emcc$defun$foo" + ], + "root": true + }, + { + "name": "emcc$defun$foo", + "reaches": [] + } +] + diff --git a/test/js_optimizer/emitDCEGraph6.js b/test/js_optimizer/emitDCEGraph6.js new file mode 100644 index 0000000000000..17ad25aea2e0d --- /dev/null +++ b/test/js_optimizer/emitDCEGraph6.js @@ -0,0 +1,35 @@ + +function foo() {} + +function bar() {} + +// caller() calls foo(). There is also another function called "caller", down +// below, which should not confuse us (if it does, nothing would refer to foo, +// and instead we'd think the toplevel caller calls bar). +function caller() { + foo(); +} + +caller(); + +var object = { + method() { + function caller(data) { + bar(); + } + } +}; + +// Similar, with an arrow function. This should also not confuse us (it would +// make "caller" refer to "arrowed". + +function arrowed() {} + +var arrow = () => { + function caller(data) { + arrowed(); + } +} + +wasmImports = {}; + From 6ece43b7f4139e467dc99f1bc459a150ee0548ff Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 13 Dec 2024 13:18:11 -0800 Subject: [PATCH 3/8] test --- test/js_optimizer/emitDCEGraph6-output.js | 1 - test/test_other.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js_optimizer/emitDCEGraph6-output.js b/test/js_optimizer/emitDCEGraph6-output.js index c70fac302fc31..28962c848bd78 100644 --- a/test/js_optimizer/emitDCEGraph6-output.js +++ b/test/js_optimizer/emitDCEGraph6-output.js @@ -21,4 +21,3 @@ "reaches": [] } ] - diff --git a/test/test_other.py b/test/test_other.py index b034bbea1f75b..e301301695d3c 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -2896,6 +2896,7 @@ def test_extern_prepost(self): 'emitDCEGraph3': (['emitDCEGraph', '--no-print'],), 'emitDCEGraph4': (['emitDCEGraph', '--no-print'],), 'emitDCEGraph5': (['emitDCEGraph', '--no-print'],), + 'emitDCEGraph6': (['emitDCEGraph', '--no-print'],), 'minimal-runtime-applyDCEGraphRemovals': (['applyDCEGraphRemovals'],), 'applyDCEGraphRemovals': (['applyDCEGraphRemovals'],), 'applyImportAndExportNameChanges': (['applyImportAndExportNameChanges'],), From f2bcfe4c1221e5317975e81de4e06d6985428d29 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 13 Dec 2024 14:35:36 -0800 Subject: [PATCH 4/8] cleanup --- test/js_optimizer/emitDCEGraph6.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/js_optimizer/emitDCEGraph6.js b/test/js_optimizer/emitDCEGraph6.js index 17ad25aea2e0d..77fe6cc52f8fa 100644 --- a/test/js_optimizer/emitDCEGraph6.js +++ b/test/js_optimizer/emitDCEGraph6.js @@ -1,4 +1,3 @@ - function foo() {} function bar() {} @@ -32,4 +31,3 @@ var arrow = () => { } wasmImports = {}; - From 0f22581e22aabb0a4c6d4990c92e7f7ffc78f654 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 13 Dec 2024 14:37:33 -0800 Subject: [PATCH 5/8] rename test file --- .../{emitDCEGraph6-output.js => emitDCEGraph-scopes-output.js} | 0 test/js_optimizer/{emitDCEGraph6.js => emitDCEGraph-scopes.js} | 0 test/test_other.py | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename test/js_optimizer/{emitDCEGraph6-output.js => emitDCEGraph-scopes-output.js} (100%) rename test/js_optimizer/{emitDCEGraph6.js => emitDCEGraph-scopes.js} (100%) diff --git a/test/js_optimizer/emitDCEGraph6-output.js b/test/js_optimizer/emitDCEGraph-scopes-output.js similarity index 100% rename from test/js_optimizer/emitDCEGraph6-output.js rename to test/js_optimizer/emitDCEGraph-scopes-output.js diff --git a/test/js_optimizer/emitDCEGraph6.js b/test/js_optimizer/emitDCEGraph-scopes.js similarity index 100% rename from test/js_optimizer/emitDCEGraph6.js rename to test/js_optimizer/emitDCEGraph-scopes.js diff --git a/test/test_other.py b/test/test_other.py index e301301695d3c..ca2d695fd5796 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -2896,7 +2896,7 @@ def test_extern_prepost(self): 'emitDCEGraph3': (['emitDCEGraph', '--no-print'],), 'emitDCEGraph4': (['emitDCEGraph', '--no-print'],), 'emitDCEGraph5': (['emitDCEGraph', '--no-print'],), - 'emitDCEGraph6': (['emitDCEGraph', '--no-print'],), + 'emitDCEGraph-scopes': (['emitDCEGraph', '--no-print'],), 'minimal-runtime-applyDCEGraphRemovals': (['applyDCEGraphRemovals'],), 'applyDCEGraphRemovals': (['applyDCEGraphRemovals'],), 'applyImportAndExportNameChanges': (['applyImportAndExportNameChanges'],), From e8da68e4f9f975efaec85dd0325868544b22eb1a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 13 Dec 2024 14:40:21 -0800 Subject: [PATCH 6/8] more asserts --- tools/acorn-optimizer.mjs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index e96a25cbe5f2a..0743e3fb2893f 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -806,6 +806,7 @@ function emitDCEGraph(ast) { emptyOut(node); // ignore this in the second pass; we scan defuns separately } } else if (node.type === 'ArrowFunctionExpression') { + assert(specialScopes > 0); specialScopes--; // Check if this is the minimal runtime exports function, which looks like // (output) => { var wasmExports = output.instance.exports; @@ -872,6 +873,7 @@ function emitDCEGraph(ast) { } } } else if (node.type === 'Property' && node.method) { + assert(specialScopes > 0); specialScopes--; } }, (node) => { From af5ab832fe9b090e4356c717a1605fbc4c4df874 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 13 Dec 2024 14:42:11 -0800 Subject: [PATCH 7/8] prettier --- tools/acorn-optimizer.mjs | 313 +++++++++++++++++++------------------- 1 file changed, 157 insertions(+), 156 deletions(-) diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index 0743e3fb2893f..6f9836259e4b0 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -710,179 +710,180 @@ function emitDCEGraph(ast) { // all content inside them as top-level, which means it is used. var specialScopes = 0; - fullWalk(ast, (node) => { - if (isWasmImportsAssign(node)) { - const assignedObject = getWasmImportsValue(node); - assignedObject.properties.forEach((item) => { - let value = item.value; - if (value.type === 'Literal' || value.type === 'FunctionExpression') { - return; // if it's a numeric or function literal, nothing to do here - } - if (value.type === 'LogicalExpression') { - // We may have something like wasmMemory || Module.wasmMemory in pthreads code; - // use the left hand identifier. - value = value.left; - } - assertAt(value.type === 'Identifier', value); - const nativeName = item.key.type == 'Literal' ? item.key.value : item.key.name; - assert(nativeName); - imports.push([value.name, nativeName]); - }); - foundWasmImportsAssign = true; - emptyOut(node); // ignore this in the second pass; this does not root - } else if (node.type === 'AssignmentExpression') { - const target = node.left; - // Ignore assignment to the wasmExports object (as happens in - // applySignatureConversions). - if (isExportUse(target)) { - emptyOut(node); - } - } else if (node.type === 'VariableDeclaration') { - if (node.declarations.length === 1) { - const item = node.declarations[0]; - const name = item.id.name; - const value = item.init; - if (value && isExportUse(value)) { - const asmName = getExportOrModuleUseName(value); - // this is: - // var _x = wasmExports['x']; - saveAsmExport(name, asmName); + fullWalk( + ast, + (node) => { + if (isWasmImportsAssign(node)) { + const assignedObject = getWasmImportsValue(node); + assignedObject.properties.forEach((item) => { + let value = item.value; + if (value.type === 'Literal' || value.type === 'FunctionExpression') { + return; // if it's a numeric or function literal, nothing to do here + } + if (value.type === 'LogicalExpression') { + // We may have something like wasmMemory || Module.wasmMemory in pthreads code; + // use the left hand identifier. + value = value.left; + } + assertAt(value.type === 'Identifier', value); + const nativeName = item.key.type == 'Literal' ? item.key.value : item.key.name; + assert(nativeName); + imports.push([value.name, nativeName]); + }); + foundWasmImportsAssign = true; + emptyOut(node); // ignore this in the second pass; this does not root + } else if (node.type === 'AssignmentExpression') { + const target = node.left; + // Ignore assignment to the wasmExports object (as happens in + // applySignatureConversions). + if (isExportUse(target)) { emptyOut(node); - } else if (value && value.type === 'ArrowFunctionExpression') { - // this is - // () => (x = wasmExports['x'])(..) - // or - // () => (x = Module['_x'] = wasmExports['x'])(..) - let asmName = isExportWrapperFunction(value); - if (asmName) { + } + } else if (node.type === 'VariableDeclaration') { + if (node.declarations.length === 1) { + const item = node.declarations[0]; + const name = item.id.name; + const value = item.init; + if (value && isExportUse(value)) { + const asmName = getExportOrModuleUseName(value); + // this is: + // var _x = wasmExports['x']; saveAsmExport(name, asmName); emptyOut(node); - } - } else if (value && value.type === 'AssignmentExpression') { - const assigned = value.left; - if (isModuleUse(assigned) && getExportOrModuleUseName(assigned) === name) { + } else if (value && value.type === 'ArrowFunctionExpression') { // this is - // var x = Module['x'] = ? - // which looks like a wasm export being received. confirm with the asm use - let found = 0; - let asmName; - fullWalk(value.right, (node) => { - if (isExportUse(node)) { - found++; - asmName = getExportOrModuleUseName(node); - } - }); - // in the wasm backend, the asm name may have one fewer "_" prefixed - if (found === 1) { - // this is indeed an export - // the asmName is what the wasm provides directly; the outside JS - // name may be slightly different (extra "_" in wasm backend) + // () => (x = wasmExports['x'])(..) + // or + // () => (x = Module['_x'] = wasmExports['x'])(..) + let asmName = isExportWrapperFunction(value); + if (asmName) { saveAsmExport(name, asmName); - emptyOut(node); // ignore this in the second pass; this does not root - return; + emptyOut(node); } - if (value.right.type === 'Literal') { + } else if (value && value.type === 'AssignmentExpression') { + const assigned = value.left; + if (isModuleUse(assigned) && getExportOrModuleUseName(assigned) === name) { // this is - // var x = Module['x'] = 1234; - // this form occurs when global addresses are exported from the - // module. It doesn't constitute a usage. - assertAt(typeof value.right.value === 'number', value.right); - emptyOut(node); + // var x = Module['x'] = ? + // which looks like a wasm export being received. confirm with the asm use + let found = 0; + let asmName; + fullWalk(value.right, (node) => { + if (isExportUse(node)) { + found++; + asmName = getExportOrModuleUseName(node); + } + }); + // in the wasm backend, the asm name may have one fewer "_" prefixed + if (found === 1) { + // this is indeed an export + // the asmName is what the wasm provides directly; the outside JS + // name may be slightly different (extra "_" in wasm backend) + saveAsmExport(name, asmName); + emptyOut(node); // ignore this in the second pass; this does not root + return; + } + if (value.right.type === 'Literal') { + // this is + // var x = Module['x'] = 1234; + // this form occurs when global addresses are exported from the + // module. It doesn't constitute a usage. + assertAt(typeof value.right.value === 'number', value.right); + emptyOut(node); + } } } } - } - // A variable declaration that has no initial values can be ignored in - // the second pass, these are just declarations, not roots - an actual - // use must be found in order to root. - if (!node.declarations.reduce((hasInit, decl) => hasInit || !!decl.init, false)) { - emptyOut(node); - } - } else if (node.type === 'FunctionDeclaration') { - if (!specialScopes) { - defuns.push(node); - const name = node.id.name; - nameToGraphName[name] = getGraphName(name, 'defun'); - emptyOut(node); // ignore this in the second pass; we scan defuns separately - } - } else if (node.type === 'ArrowFunctionExpression') { - assert(specialScopes > 0); - specialScopes--; - // Check if this is the minimal runtime exports function, which looks like - // (output) => { var wasmExports = output.instance.exports; - if ( - node.params.length === 1 && - node.params[0].type === 'Identifier' && - node.params[0].name === 'output' && - node.body.type === 'BlockStatement' - ) { - const body = node.body.body; - if (body.length >= 1) { - const first = body[0]; - let target; - let value; // "(var?) target = value" - // Look either for var wasmExports = or just wasmExports = - if (first.type === 'VariableDeclaration' && first.declarations.length === 1) { - const decl = first.declarations[0]; - target = decl.id; - value = decl.init; - } else if ( - first.type === 'ExpressionStatement' && - first.expression.type === 'AssignmentExpression' - ) { - const assign = first.expression; - if (assign.operator === '=') { - target = assign.left; - value = assign.right; - } - } - if (target && target.type === 'Identifier' && target.name === 'wasmExports' && value) { - if ( - value.type === 'MemberExpression' && - value.object.type === 'MemberExpression' && - value.object.object.type === 'Identifier' && - value.object.object.name === 'output' && - value.object.property.type === 'Identifier' && - value.object.property.name === 'instance' && - value.property.type === 'Identifier' && - value.property.name === 'exports' + // A variable declaration that has no initial values can be ignored in + // the second pass, these are just declarations, not roots - an actual + // use must be found in order to root. + if (!node.declarations.reduce((hasInit, decl) => hasInit || !!decl.init, false)) { + emptyOut(node); + } + } else if (node.type === 'FunctionDeclaration') { + if (!specialScopes) { + defuns.push(node); + const name = node.id.name; + nameToGraphName[name] = getGraphName(name, 'defun'); + emptyOut(node); // ignore this in the second pass; we scan defuns separately + } + } else if (node.type === 'ArrowFunctionExpression') { + specialScopes--; + // Check if this is the minimal runtime exports function, which looks like + // (output) => { var wasmExports = output.instance.exports; + if ( + node.params.length === 1 && + node.params[0].type === 'Identifier' && + node.params[0].name === 'output' && + node.body.type === 'BlockStatement' + ) { + const body = node.body.body; + if (body.length >= 1) { + const first = body[0]; + let target; + let value; // "(var?) target = value" + // Look either for var wasmExports = or just wasmExports = + if (first.type === 'VariableDeclaration' && first.declarations.length === 1) { + const decl = first.declarations[0]; + target = decl.id; + value = decl.init; + } else if ( + first.type === 'ExpressionStatement' && + first.expression.type === 'AssignmentExpression' ) { - // This looks very much like what we are looking for. - assert(!foundMinimalRuntimeExports); - for (let i = 1; i < body.length; i++) { - const item = body[i]; - if ( - item.type === 'ExpressionStatement' && - item.expression.type === 'AssignmentExpression' && - item.expression.operator === '=' && - item.expression.left.type === 'Identifier' && - item.expression.right.type === 'MemberExpression' && - item.expression.right.object.type === 'Identifier' && - item.expression.right.object.name === 'wasmExports' && - item.expression.right.property.type === 'Literal' - ) { - const name = item.expression.left.name; - const asmName = item.expression.right.property.value; - saveAsmExport(name, asmName); - emptyOut(item); // ignore all this in the second pass; this does not root + const assign = first.expression; + if (assign.operator === '=') { + target = assign.left; + value = assign.right; + } + } + if (target && target.type === 'Identifier' && target.name === 'wasmExports' && value) { + if ( + value.type === 'MemberExpression' && + value.object.type === 'MemberExpression' && + value.object.object.type === 'Identifier' && + value.object.object.name === 'output' && + value.object.property.type === 'Identifier' && + value.object.property.name === 'instance' && + value.property.type === 'Identifier' && + value.property.name === 'exports' + ) { + // This looks very much like what we are looking for. + assert(!foundMinimalRuntimeExports); + for (let i = 1; i < body.length; i++) { + const item = body[i]; + if ( + item.type === 'ExpressionStatement' && + item.expression.type === 'AssignmentExpression' && + item.expression.operator === '=' && + item.expression.left.type === 'Identifier' && + item.expression.right.type === 'MemberExpression' && + item.expression.right.object.type === 'Identifier' && + item.expression.right.object.name === 'wasmExports' && + item.expression.right.property.type === 'Literal' + ) { + const name = item.expression.left.name; + const asmName = item.expression.right.property.value; + saveAsmExport(name, asmName); + emptyOut(item); // ignore all this in the second pass; this does not root + } } + foundMinimalRuntimeExports = true; } - foundMinimalRuntimeExports = true; } } } + } else if (node.type === 'Property' && node.method) { + specialScopes--; } - } else if (node.type === 'Property' && node.method) { - assert(specialScopes > 0); - specialScopes--; - } - }, (node) => { - // Pre-walking logic. We note special scopes (see above). - if (node.type === 'ArrowFunctionExpression' || - (node.type === 'Property' && node.method)) { - specialScopes++; - } - }); + }, + (node) => { + // Pre-walking logic. We note special scopes (see above). + if (node.type === 'ArrowFunctionExpression' || (node.type === 'Property' && node.method)) { + specialScopes++; + } + }, + ); // Scoping must balance out. assert(specialScopes === 0); // We must have found the info we need. From 33447f3e6f67418fcfc0470ea63149eaafa21890 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 13 Dec 2024 14:44:15 -0800 Subject: [PATCH 8/8] asserts --- tools/acorn-optimizer.mjs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index 6f9836259e4b0..a6932295e1f5c 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -808,6 +808,7 @@ function emitDCEGraph(ast) { emptyOut(node); // ignore this in the second pass; we scan defuns separately } } else if (node.type === 'ArrowFunctionExpression') { + assert(specialScopes > 0); specialScopes--; // Check if this is the minimal runtime exports function, which looks like // (output) => { var wasmExports = output.instance.exports; @@ -874,6 +875,7 @@ function emitDCEGraph(ast) { } } } else if (node.type === 'Property' && node.method) { + assert(specialScopes > 0); specialScopes--; } },