diff --git a/scripts/fuzz_opt.py b/scripts/fuzz_opt.py index 8e80c2bb9fc..e3225f298a6 100755 --- a/scripts/fuzz_opt.py +++ b/scripts/fuzz_opt.py @@ -188,7 +188,7 @@ def randomize_fuzz_settings(): FUZZ_OPTS += ['--no-fuzz-oob'] if random.random() < 0.5: LEGALIZE = True - FUZZ_OPTS += ['--legalize-js-interface'] + FUZZ_OPTS += ['--legalize-and-prune-js-interface'] else: LEGALIZE = False @@ -924,7 +924,7 @@ def compare_before_and_after(self, before, after): compare(before[vm], after[vm], 'CompareVMs between before and after: ' + vm.name) def can_run_on_feature_opts(self, feature_opts): - return all_disallowed(['simd', 'multivalue', 'multimemory']) + return True # Check for determinism - the same command must have the same output. @@ -955,7 +955,7 @@ def handle_pair(self, input, before_wasm, after_wasm, opts): # later make sense (if we don't do this, the wasm may have i64 exports). # after applying other necessary fixes, we'll recreate the after wasm # from scratch. - run([in_bin('wasm-opt'), before_wasm, '--legalize-js-interface', '-o', before_wasm_temp] + FEATURE_OPTS) + run([in_bin('wasm-opt'), before_wasm, '--legalize-and-prune-js-interface', '-o', before_wasm_temp] + FEATURE_OPTS) compare_before_to_after = random.random() < 0.5 compare_to_interpreter = compare_before_to_after and random.random() < 0.5 if compare_before_to_after: diff --git a/scripts/fuzz_shell.js b/scripts/fuzz_shell.js index 4a3b42d9f14..2d038af5f84 100644 --- a/scripts/fuzz_shell.js +++ b/scripts/fuzz_shell.js @@ -54,6 +54,11 @@ var imports = { 'log-i64': logValue, 'log-f32': logValue, 'log-f64': logValue, + // JS cannot log v128 values (we trap on the boundary), but we must still + // provide an import so that we do not trap during linking. (Alternatively, + // we could avoid running JS on code with SIMD in it, but it is useful to + // fuzz such code as much as we can.) + 'log-v128': logValue, }, 'env': { 'setTempRet0': function(x) { tempRet0 = x }, diff --git a/src/passes/LegalizeJSInterface.cpp b/src/passes/LegalizeJSInterface.cpp index 3ca6b7095e0..0082ba7ab5f 100644 --- a/src/passes/LegalizeJSInterface.cpp +++ b/src/passes/LegalizeJSInterface.cpp @@ -29,6 +29,12 @@ // across modules, we still want to legalize dynCalls so JS can call into the // tables even to a signature that is not legal. // +// Another variation also "prunes" imports and exports that we cannot yet +// legalize, like exports and imports with SIMD or multivalue. Until we write +// the logic to legalize them, removing those imports/exports still allows us to +// fuzz all the legal imports/exports. (Note that multivalue is supported in +// exports in newer VMs - node 16+ - so that part is only needed for older VMs.) +// #include "asmjs/shared-constants.h" #include "ir/element-utils.h" @@ -43,6 +49,8 @@ namespace wasm { +namespace { + // These are aliases for getTempRet0/setTempRet0 which emscripten defines in // compiler-rt and exports under these names. static Name GET_TEMP_RET_EXPORT("__get_temp_ret"); @@ -358,10 +366,88 @@ struct LegalizeJSInterface : public Pass { } }; +struct LegalizeAndPruneJSInterface : public LegalizeJSInterface { + // Legalize fully (true) and add pruning on top. + LegalizeAndPruneJSInterface() : LegalizeJSInterface(true) {} + + void run(Module* module) override { + LegalizeJSInterface::run(module); + + prune(module); + } + + void prune(Module* module) { + // For each function name, the exported id it is exported with. For + // example, + // + // (func $foo (export "bar") + // + // Would have exportedFunctions["foo"] = "bar"; + std::unordered_map exportedFunctions; + for (auto& exp : module->exports) { + if (exp->kind == ExternalKind::Function) { + exportedFunctions[exp->value] = exp->name; + } + } + + for (auto& func : module->functions) { + // If the function is neither exported nor imported, no problem. + auto imported = func->imported(); + auto exported = exportedFunctions.count(func->name); + if (!imported && !exported) { + continue; + } + + // The params are allowed to be multivalue, but not the results. Otherwise + // look for SIMD. + auto sig = func->type.getSignature(); + auto illegal = isIllegal(sig.results); + illegal = + illegal || std::any_of(sig.params.begin(), + sig.params.end(), + [&](const Type& t) { return isIllegal(t); }); + if (!illegal) { + continue; + } + + // Prune an import by implementing it in a trivial manner. + if (imported) { + func->module = func->base = Name(); + + Builder builder(*module); + if (sig.results == Type::none) { + func->body = builder.makeNop(); + } else { + func->body = + builder.makeConstantExpression(Literal::makeZeros(sig.results)); + } + } + + // Prune an export by just removing it. + if (exported) { + module->removeExport(exportedFunctions[func->name]); + } + } + + // TODO: globals etc. + } + + bool isIllegal(Type type) { + auto features = type.getFeatures(); + return features.hasSIMD() || features.hasMultivalue(); + } +}; + +} // anonymous namespace + Pass* createLegalizeJSInterfacePass() { return new LegalizeJSInterface(true); } Pass* createLegalizeJSInterfaceMinimallyPass() { return new LegalizeJSInterface(false); } +Pass* createLegalizeAndPruneJSInterfacePass() { + return new LegalizeAndPruneJSInterface(); +} + } // namespace wasm diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 1fe81cbfc7d..6faf77276a6 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -221,6 +221,9 @@ void PassRegistry::registerPasses() { "legalizes i64 types on the import/export boundary in a minimal " "manner, only on things only JS will call", createLegalizeJSInterfaceMinimallyPass); + registerPass("legalize-and-prune-js-interface", + "legalizes the import/export boundary and prunes when needed", + createLegalizeAndPruneJSInterfacePass); registerPass("local-cse", "common subexpression elimination inside basic blocks", createLocalCSEPass); diff --git a/src/passes/passes.h b/src/passes/passes.h index c3ab7773f1e..2f188a689ad 100644 --- a/src/passes/passes.h +++ b/src/passes/passes.h @@ -67,6 +67,7 @@ Pass* createInliningPass(); Pass* createInliningOptimizingPass(); Pass* createJSPIPass(); Pass* createJ2CLOptsPass(); +Pass* createLegalizeAndPruneJSInterfacePass(); Pass* createLegalizeJSInterfacePass(); Pass* createLegalizeJSInterfaceMinimallyPass(); Pass* createLimitSegmentsPass(); diff --git a/test/lit/help/wasm-opt.test b/test/lit/help/wasm-opt.test index be5924cf7fe..99050baa5cf 100644 --- a/test/lit/help/wasm-opt.test +++ b/test/lit/help/wasm-opt.test @@ -229,6 +229,9 @@ ;; CHECK-NEXT: --jspi wrap imports and exports for ;; CHECK-NEXT: JavaScript promise integration ;; CHECK-NEXT: +;; CHECK-NEXT: --legalize-and-prune-js-interface legalizes the import/export +;; CHECK-NEXT: boundary and prunes when needed +;; CHECK-NEXT: ;; CHECK-NEXT: --legalize-js-interface legalizes i64 types on the ;; CHECK-NEXT: import/export boundary ;; CHECK-NEXT: diff --git a/test/lit/help/wasm2js.test b/test/lit/help/wasm2js.test index aadefe8fc9b..9804718a819 100644 --- a/test/lit/help/wasm2js.test +++ b/test/lit/help/wasm2js.test @@ -179,6 +179,9 @@ ;; CHECK-NEXT: --jspi wrap imports and exports for ;; CHECK-NEXT: JavaScript promise integration ;; CHECK-NEXT: +;; CHECK-NEXT: --legalize-and-prune-js-interface legalizes the import/export +;; CHECK-NEXT: boundary and prunes when needed +;; CHECK-NEXT: ;; CHECK-NEXT: --legalize-js-interface legalizes i64 types on the ;; CHECK-NEXT: import/export boundary ;; CHECK-NEXT: diff --git a/test/lit/passes/legalize-and-prune-js-interface.wast b/test/lit/passes/legalize-and-prune-js-interface.wast new file mode 100644 index 00000000000..1683818a2d8 --- /dev/null +++ b/test/lit/passes/legalize-and-prune-js-interface.wast @@ -0,0 +1,212 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. +;; NOTE: This test was ported using port_passes_tests_to_lit.py and could be cleaned up. + +;; RUN: foreach %s %t wasm-opt -all --legalize-and-prune-js-interface -S -o - | filecheck %s + +(module + (import "env" "imported-64" (func $imported-64 (param i32 f64) (result i64))) + + (import "env" "imported-v128" (func $imported-v128 (result v128))) + + (import "env" "imported-mv" (func $imported-mv (result i32 f64))) + + (import "env" "imported-v128-param" (func $imported-v128-param (param v128) (result i32))) + + (import "env" "imported-v128-param-noresult" (func $imported-v128-param-noresult (param v128))) + + ;; CHECK: (type $0 (func (result v128))) + + ;; CHECK: (type $1 (func (result i32 f64))) + + ;; CHECK: (type $2 (func (param v128) (result i32))) + + ;; CHECK: (type $3 (func (param v128))) + + ;; CHECK: (type $4 (func)) + + ;; CHECK: (type $5 (func (result i32))) + + ;; CHECK: (type $6 (func (param i32 f64) (result i64))) + + ;; CHECK: (type $7 (func (param i32 f64) (result i32))) + + ;; CHECK: (import "env" "getTempRet0" (func $getTempRet0 (type $5) (result i32))) + + ;; CHECK: (import "env" "imported-64" (func $legalimport$imported-64 (type $7) (param i32 f64) (result i32))) + + ;; CHECK: (func $imported-v128 (type $0) (result v128) + ;; CHECK-NEXT: (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000) + ;; CHECK-NEXT: ) + + ;; CHECK: (func $imported-mv (type $1) (result i32 f64) + ;; CHECK-NEXT: (tuple.make 2 + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (f64.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + + ;; CHECK: (func $imported-v128-param (type $2) (param $0 v128) (result i32) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + + ;; CHECK: (func $imported-v128-param-noresult (type $3) (param $0 v128) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + + ;; CHECK: (func $call-64 (type $4) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $legalfunc$imported-64 + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (f64.const 1.2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $imported-v128) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (tuple.drop 2 + ;; CHECK-NEXT: (call $imported-mv) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $imported-v128-param + ;; CHECK-NEXT: (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $imported-v128-param-noresult + ;; CHECK-NEXT: (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-64 + ;; This import has an i64 which will be legalized, the same as in the + ;; normal --legalize-js-interface pass. Note how the multiple params are not + ;; an issue for us. + (drop (call $imported-64 + (i32.const 0) + (f64.const 1.2) + )) + + ;; This import uses SIMD, which we must prune from the list of imports. + ;; We'll define it in a trivial manner inside the module instead. + (drop (call $imported-v128)) + + ;; Ditto, but with multivalue. + (tuple.drop 2 (call $imported-mv)) + + ;; Ditto, but now the illegal thing is a param. + (drop (call $imported-v128-param + (v128.const i32x4 0 0 0 0) + )) + + ;; Ditto, but no result this time. + (call $imported-v128-param-noresult + (v128.const i32x4 0 0 0 0) + ) + ) +) + +;; CHECK: (func $legalfunc$imported-64 (type $6) (param $0 i32) (param $1 f64) (result i64) +;; CHECK-NEXT: (i64.or +;; CHECK-NEXT: (i64.extend_i32_u +;; CHECK-NEXT: (call $legalimport$imported-64 +;; CHECK-NEXT: (local.get $0) +;; CHECK-NEXT: (local.get $1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i64.shl +;; CHECK-NEXT: (i64.extend_i32_u +;; CHECK-NEXT: (call $getTempRet0) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i64.const 32) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +(module + ;; CHECK: (type $0 (func (param i64) (result i64))) + + ;; CHECK: (type $1 (func (param v128))) + + ;; CHECK: (type $2 (func (result v128))) + + ;; CHECK: (type $3 (func (result i32 i32))) + + ;; CHECK: (type $4 (func (param i32))) + + ;; CHECK: (type $5 (func (param i32 i32) (result i32))) + + ;; CHECK: (import "env" "setTempRet0" (func $setTempRet0 (type $4) (param i32))) + + ;; CHECK: (export "export-64" (func $legalstub$export-64)) + + ;; CHECK: (func $export-64 (type $0) (param $x i64) (result i64) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $export-64 (export "export-64") (param $x i64) (result i64) + ;; This can be legalized. Note we have two params, but that's no problem. + (unreachable) + ) + + ;; CHECK: (func $export-v128 (type $1) (param $x v128) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $export-v128 (export "export-v128") (param $x v128) + ;; This will be pruned. + (unreachable) + ) + + ;; CHECK: (func $export-v128-result (type $2) (result v128) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $export-v128-result (export "export-v128-result") (result v128) + ;; This will be pruned. + (unreachable) + ) + + ;; CHECK: (func $export-mv (type $3) (result i32 i32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $export-mv (export "export-mv") (result i32 i32) + ;; This will be pruned. + (unreachable) + ) +) + +;; CHECK: (func $legalstub$export-64 (type $5) (param $0 i32) (param $1 i32) (result i32) +;; CHECK-NEXT: (local $2 i64) +;; CHECK-NEXT: (local.set $2 +;; CHECK-NEXT: (call $export-64 +;; CHECK-NEXT: (i64.or +;; CHECK-NEXT: (i64.extend_i32_u +;; CHECK-NEXT: (local.get $0) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i64.shl +;; CHECK-NEXT: (i64.extend_i32_u +;; CHECK-NEXT: (local.get $1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i64.const 32) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (call $setTempRet0 +;; CHECK-NEXT: (i32.wrap_i64 +;; CHECK-NEXT: (i64.shr_u +;; CHECK-NEXT: (local.get $2) +;; CHECK-NEXT: (i64.const 32) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i32.wrap_i64 +;; CHECK-NEXT: (local.get $2) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +(module + (import "env" "imported-v128" (func $imported-v128 (result v128))) + + ;; The import is also exported. We will both implement it with a trivial body + ;; and also prune the export, so it remains neither an import nor an export. + (export "imported-v128" (func $imported-v128)) +) +;; CHECK: (type $0 (func (result v128))) + +;; CHECK: (func $imported-v128 (type $0) (result v128) +;; CHECK-NEXT: (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000) +;; CHECK-NEXT: )