diff options
author | Thomas Lively <tlively@google.com> | 2022-12-20 13:41:04 -0600 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-20 19:41:04 +0000 |
commit | 8c2696b78f5658888d0d480eaddd9eed045b3e7b (patch) | |
tree | 6010ca853999c110c4530af535e07ca253c9e671 | |
parent | 569f789622f116177c8a1e32fb62a4e5a5c9dfe0 (diff) | |
download | binaryen-8c2696b78f5658888d0d480eaddd9eed045b3e7b.tar.gz binaryen-8c2696b78f5658888d0d480eaddd9eed045b3e7b.tar.bz2 binaryen-8c2696b78f5658888d0d480eaddd9eed045b3e7b.zip |
Work around bugs with open world type optimizations (#5367)
Since #5347 public types are never updated by type optimizations, but the
optimization passes have not yet been updated to take that into account, so they
are all buggy under an open world assumption. In #5359 we worked around many
closed world validation errors in the fuzzer by treating --closed-world like a
feature flag and checking whether it was necessary for fuzzer input, but that
did not prevent the type optimization passes from running under an open world,
so it did not work around all the potential issues.
Work around the problem more thoroughly by not running any type optimization
passes in the fuzzer without --closed-world. Also add logic to those passes to
error out if they are run without --closed-world and update the tests
accordingly.
-rwxr-xr-x | scripts/fuzz_opt.py | 158 | ||||
-rw-r--r-- | src/ir/module-utils.cpp | 5 | ||||
-rw-r--r-- | src/passes/GlobalTypeOptimization.cpp | 4 | ||||
-rw-r--r-- | src/passes/RemoveUnusedTypes.cpp | 10 | ||||
-rw-r--r-- | src/passes/SignaturePruning.cpp | 4 | ||||
-rw-r--r-- | src/passes/TypeMerging.cpp | 4 | ||||
-rw-r--r-- | src/passes/TypeRefining.cpp | 6 | ||||
-rw-r--r-- | test/lit/exec/no-compare-refs.wast | 2 | ||||
-rw-r--r-- | test/lit/passes/gto-mutability.wast | 2 | ||||
-rw-r--r-- | test/lit/passes/gto-removals.wast | 4 | ||||
-rw-r--r-- | test/lit/passes/remove-unused-types.wast | 2 | ||||
-rw-r--r-- | test/lit/passes/signature-pruning.wast | 2 | ||||
-rw-r--r-- | test/lit/passes/signature-refining_gto.wat | 4 | ||||
-rw-r--r-- | test/lit/passes/type-merging.wast | 2 | ||||
-rw-r--r-- | test/lit/passes/type-refining-isorecursive.wast | 2 | ||||
-rw-r--r-- | test/lit/passes/type-refining.wast | 2 | ||||
-rw-r--r-- | test/lit/passes/type-ssa_and_merging.wast | 16 |
17 files changed, 132 insertions, 97 deletions
diff --git a/scripts/fuzz_opt.py b/scripts/fuzz_opt.py index 9da15cd64..57bb4122b 100755 --- a/scripts/fuzz_opt.py +++ b/scripts/fuzz_opt.py @@ -1271,85 +1271,105 @@ def write_commands(commands, filename): # main opt_choices = [ - [], - ['-O1'], ['-O2'], ['-O3'], ['-O4'], ['-Os'], ['-Oz'], - ["--cfp"], - ["--coalesce-locals"], - # XXX slow, non-default ["--coalesce-locals-learning"], - ["--code-pushing"], - ["--code-folding"], - ["--const-hoisting"], - ["--dae"], - ["--dae-optimizing"], - ["--dce"], - ["--directize"], - ["--discard-global-effects"], - ["--flatten", "--dfo"], - ["--duplicate-function-elimination"], - ["--flatten"], - # ["--fpcast-emu"], # removes indirect call failures as it makes them go through regardless of type - ["--inlining"], - ["--inlining-optimizing"], - ["--flatten", "--simplify-locals-notee-nostructure", "--local-cse"], + (), + ('-O1',), ('-O2',), ('-O3',), ('-O4',), ('-Os',), ('-Oz',), + ("--cfp",), + ("--coalesce-locals",), + # XXX slow, non-default ("--coalesce-locals-learning",), + ("--code-pushing",), + ("--code-folding",), + ("--const-hoisting",), + ("--dae",), + ("--dae-optimizing",), + ("--dce",), + ("--directize",), + ("--discard-global-effects",), + ("--flatten", "--dfo",), + ("--duplicate-function-elimination",), + ("--flatten",), + # ("--fpcast-emu",), # removes indirect call failures as it makes them go through regardless of type + ("--inlining",), + ("--inlining-optimizing",), + ("--flatten", "--simplify-locals-notee-nostructure", "--local-cse",), # note that no pass we run here should add effects to a function, so it is # ok to run this pass and let the passes after it use the effects to # optimize - ["--generate-global-effects"], - ["--global-refining"], - ["--gsi"], - ["--gto"], - ["--gufa"], - ["--gufa-optimizing"], - ["--local-cse"], - ["--heap2local"], - ["--remove-unused-names", "--heap2local"], - ["--generate-stack-ir"], - ["--licm"], - ["--local-subtyping"], - ["--memory-packing"], - ["--merge-blocks"], - ['--merge-locals'], - ['--monomorphize'], - ['--monomorphize-always'], - ['--once-reduction'], - ["--optimize-casts"], - ["--optimize-instructions"], - ["--optimize-stack-ir"], - ["--generate-stack-ir", "--optimize-stack-ir"], - ["--pick-load-signs"], - ["--precompute"], - ["--precompute-propagate"], - ["--print"], - ["--remove-unused-brs"], - ["--remove-unused-nonfunction-module-elements"], - ["--remove-unused-module-elements"], - ["--remove-unused-names"], - ["--reorder-functions"], - ["--reorder-locals"], - ["--flatten", "--rereloop"], - ["--roundtrip"], - ["--rse"], - ["--signature-pruning"], - ["--signature-refining"], - ["--simplify-locals"], - ["--simplify-locals-nonesting"], - ["--simplify-locals-nostructure"], - ["--simplify-locals-notee"], - ["--simplify-locals-notee-nostructure"], - ["--ssa"], - ["--type-refining"], - ["--type-merging"], - ["--type-ssa"], - ["--vacuum"], + ("--generate-global-effects",), + ("--global-refining",), + ("--gsi",), + ("--gto",), + ("--gufa",), + ("--gufa-optimizing",), + ("--local-cse",), + ("--heap2local",), + ("--remove-unused-names", "--heap2local",), + ("--generate-stack-ir",), + ("--licm",), + ("--local-subtyping",), + ("--memory-packing",), + ("--merge-blocks",), + ('--merge-locals',), + ('--monomorphize',), + ('--monomorphize-always',), + ('--once-reduction',), + ("--optimize-casts",), + ("--optimize-instructions",), + ("--optimize-stack-ir",), + ("--generate-stack-ir", "--optimize-stack-ir",), + ("--pick-load-signs",), + ("--precompute",), + ("--precompute-propagate",), + ("--print",), + ("--remove-unused-brs",), + ("--remove-unused-nonfunction-module-elements",), + ("--remove-unused-module-elements",), + ("--remove-unused-names",), + ("--remove-unused-types",), + ("--reorder-functions",), + ("--reorder-locals",), + ("--flatten", "--rereloop",), + ("--roundtrip",), + ("--rse",), + ("--signature-pruning",), + ("--signature-refining",), + ("--simplify-locals",), + ("--simplify-locals-nonesting",), + ("--simplify-locals-nostructure",), + ("--simplify-locals-notee",), + ("--simplify-locals-notee-nostructure",), + ("--ssa",), + ("--type-refining",), + ("--type-merging",), + ("--type-ssa",), + ("--vacuum",), ] +# TODO: Fix these passes so that they still work without --closed-world! +requires_closed_world = {("--type-refining",), + ("--signature-pruning",), + ("--signature-refining",), + ("--gto",), + ("--remove-unused-types",), + ("--cfp",), + ("--gsi",), + ("--type-ssa",), + ("--type-merging",)} + def randomize_opt_flags(): flag_groups = [] has_flatten = False + + if CLOSED_WORLD: + usable_opt_choices = opt_choices + else: + usable_opt_choices = [choice + for choice in opt_choices + if choice not in requires_closed_world] + # core opts while 1: - choice = random.choice(opt_choices) + choice = random.choice(usable_opt_choices) if '--flatten' in choice or '-O4' in choice: if has_flatten: print('avoiding multiple --flatten in a single command, due to exponential overhead') @@ -1376,7 +1396,7 @@ def randomize_opt_flags(): # maybe add an extra round trip if random.random() < 0.5: pos = random.randint(0, len(flag_groups)) - flag_groups = flag_groups[:pos] + [['--roundtrip']] + flag_groups[pos:] + flag_groups = flag_groups[:pos] + [('--roundtrip',)] + flag_groups[pos:] ret = [flag for group in flag_groups for flag in group] # modifiers (if not already implied by a -O? option) if '-O' not in str(ret): diff --git a/src/ir/module-utils.cpp b/src/ir/module-utils.cpp index 8e6ac0d1a..f7ba7d776 100644 --- a/src/ir/module-utils.cpp +++ b/src/ir/module-utils.cpp @@ -317,11 +317,10 @@ std::vector<HeapType> getPublicHeapTypes(Module& wasm) { } std::vector<HeapType> getPrivateHeapTypes(Module& wasm) { - auto allTypes = getHeapTypeCounts(wasm, true); + auto usedTypes = getHeapTypeCounts(wasm, true); auto publicTypes = getPublicTypeSet(wasm); std::vector<HeapType> types; - types.reserve(allTypes.size() - publicTypes.size()); - for (auto& [type, _] : allTypes) { + for (auto& [type, _] : usedTypes) { if (!publicTypes.count(type)) { types.push_back(type); } diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 9015f3ab5..8ef6a3b89 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -118,6 +118,10 @@ struct GlobalTypeOptimization : public Pass { return; } + if (!getPassOptions().closedWorld) { + Fatal() << "GTO requires --closed-world"; + } + // Find and analyze struct operations inside each function. StructUtils::FunctionStructValuesMap<FieldInfo> functionNewInfos(*module), functionSetGetInfos(*module); diff --git a/src/passes/RemoveUnusedTypes.cpp b/src/passes/RemoveUnusedTypes.cpp index d346bc585..1ea65fb0f 100644 --- a/src/passes/RemoveUnusedTypes.cpp +++ b/src/passes/RemoveUnusedTypes.cpp @@ -34,6 +34,16 @@ struct RemoveUnusedTypes : Pass { // This pass only does anything with GC types. return; } + + // Consider (rec $A $unused), where anyrefs received from the outside are + // cast to `$A`. In an open world we cannot remove $unused because that + // would change the identity of $A. Currently we would incorrectly remove + // $unused. To fix that, we need to fix our collection of public types to + // consider $A (and $unused) public in an open world. + if (!getPassOptions().closedWorld) { + Fatal() << "RemoveUnusedTypes requires --closed-world"; + } + // We're not changing the contents of any of the types, so we just round // trip them throgh GlobalTypeRewriter, which will put all the private types // in a single new rec group and leave out all the unused types. diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp index 3244ee103..168ef8454 100644 --- a/src/passes/SignaturePruning.cpp +++ b/src/passes/SignaturePruning.cpp @@ -53,6 +53,10 @@ struct SignaturePruning : public Pass { return; } + if (!getPassOptions().closedWorld) { + Fatal() << "SignaturePruning requires --closed-world"; + } + if (!module->tables.empty()) { // When there are tables we must also take their types into account, which // would require us to take call_indirect, element segments, etc. into diff --git a/src/passes/TypeMerging.cpp b/src/passes/TypeMerging.cpp index d873152f2..c5028ee20 100644 --- a/src/passes/TypeMerging.cpp +++ b/src/passes/TypeMerging.cpp @@ -109,6 +109,10 @@ struct TypeMerging : public Pass { return; } + if (!getPassOptions().closedWorld) { + Fatal() << "TypeMerging requires --closed-world"; + } + // First, find all the cast types. ModuleUtils::ParallelFunctionAnalysis<ReferredTypes> analysis( diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp index 6e2c96fce..0938f6d8e 100644 --- a/src/passes/TypeRefining.cpp +++ b/src/passes/TypeRefining.cpp @@ -103,9 +103,9 @@ struct TypeRefining : public Pass { if (!module->features.hasGC()) { return; } - if (getTypeSystem() != TypeSystem::Nominal && - getTypeSystem() != TypeSystem::Isorecursive) { - Fatal() << "TypeRefining requires nominal/hybrid typing"; + + if (!getPassOptions().closedWorld) { + Fatal() << "TypeRefining requires --closed-world"; } // Find and analyze struct operations inside each function. diff --git a/test/lit/exec/no-compare-refs.wast b/test/lit/exec/no-compare-refs.wast index bb9766a31..d456ab118 100644 --- a/test/lit/exec/no-compare-refs.wast +++ b/test/lit/exec/no-compare-refs.wast @@ -1,6 +1,6 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --output=fuzz-exec and should not be edited. -;; RUN: wasm-opt %s --hybrid -all --signature-pruning --fuzz-exec -q -o /dev/null 2>&1 | filecheck %s +;; RUN: wasm-opt %s --hybrid -all --signature-pruning --closed-world --fuzz-exec -q -o /dev/null 2>&1 | filecheck %s (module (type $f (func (param i32))) diff --git a/test/lit/passes/gto-mutability.wast b/test/lit/passes/gto-mutability.wast index 4f1e9a6d4..32d20b450 100644 --- a/test/lit/passes/gto-mutability.wast +++ b/test/lit/passes/gto-mutability.wast @@ -1,5 +1,5 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: foreach %s %t wasm-opt --nominal --gto -all -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt --nominal --gto --closed-world -all -S -o - | filecheck %s ;; (remove-unused-names is added to test fallthrough values without a block ;; name getting in the way) diff --git a/test/lit/passes/gto-removals.wast b/test/lit/passes/gto-removals.wast index b5890e6b1..c2895cf32 100644 --- a/test/lit/passes/gto-removals.wast +++ b/test/lit/passes/gto-removals.wast @@ -1,7 +1,5 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: foreach %s %t wasm-opt --nominal --gto -all -S -o - | filecheck %s -;; (remove-unused-names is added to test fallthrough values without a block -;; name getting in the way) +;; RUN: foreach %s %t wasm-opt --nominal --gto --closed-world -all -S -o - | filecheck %s (module ;; A struct with a field that is never read or written, so it can be diff --git a/test/lit/passes/remove-unused-types.wast b/test/lit/passes/remove-unused-types.wast index 5dcf889e4..afcafe8cd 100644 --- a/test/lit/passes/remove-unused-types.wast +++ b/test/lit/passes/remove-unused-types.wast @@ -1,6 +1,6 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: wasm-opt %s --remove-unused-types -all -S -o - | filecheck %s +;; RUN: wasm-opt %s --remove-unused-types --closed-world -all -S -o - | filecheck %s (module (rec diff --git a/test/lit/passes/signature-pruning.wast b/test/lit/passes/signature-pruning.wast index 493b7237a..c3d6567f7 100644 --- a/test/lit/passes/signature-pruning.wast +++ b/test/lit/passes/signature-pruning.wast @@ -1,5 +1,5 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: foreach %s %t wasm-opt --nominal --signature-pruning -all -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt --nominal --signature-pruning --closed-world -all -S -o - | filecheck %s (module ;; CHECK: (type $sig (func (param i32 f64))) diff --git a/test/lit/passes/signature-refining_gto.wat b/test/lit/passes/signature-refining_gto.wat index 2a6f768c8..8e034ef3e 100644 --- a/test/lit/passes/signature-refining_gto.wat +++ b/test/lit/passes/signature-refining_gto.wat @@ -1,5 +1,5 @@ -;; RUN: foreach %s %t wasm-opt --nominal --signature-refining --gto --roundtrip -all -S -o - | filecheck %s -;; RUN: foreach %s %t wasm-opt --signature-refining --gto --remove-unused-types --roundtrip -all -S -o - | filecheck %s --check-prefix ISOREC +;; RUN: foreach %s %t wasm-opt --closed-world --nominal --signature-refining --gto --roundtrip -all -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt --closed-world --signature-refining --gto --remove-unused-types --roundtrip -all -S -o - | filecheck %s --check-prefix ISOREC ;; Check that type $A is not included in the final binary after the signature ;; refining optimization. For isorecursive types, this requires an additional diff --git a/test/lit/passes/type-merging.wast b/test/lit/passes/type-merging.wast index 56e339777..85565551d 100644 --- a/test/lit/passes/type-merging.wast +++ b/test/lit/passes/type-merging.wast @@ -1,5 +1,5 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: foreach %s %t wasm-opt --nominal --type-merging -all -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt --nominal --closed-world --type-merging -all -S -o - | filecheck %s (module ;; CHECK: (type $A (struct (field i32))) diff --git a/test/lit/passes/type-refining-isorecursive.wast b/test/lit/passes/type-refining-isorecursive.wast index 820e6283a..245801843 100644 --- a/test/lit/passes/type-refining-isorecursive.wast +++ b/test/lit/passes/type-refining-isorecursive.wast @@ -1,5 +1,5 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: foreach %s %t wasm-opt --hybrid --type-refining -all -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt --hybrid --type-refining --closed-world -all -S -o - | filecheck %s (module ;; The types should be refined to a set of three mutually recursive types. diff --git a/test/lit/passes/type-refining.wast b/test/lit/passes/type-refining.wast index 62f8b273c..5356d7a0f 100644 --- a/test/lit/passes/type-refining.wast +++ b/test/lit/passes/type-refining.wast @@ -1,5 +1,5 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: foreach %s %t wasm-opt --nominal --type-refining -all -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt --nominal --closed-world --type-refining -all -S -o - | filecheck %s (module ;; A struct with three fields. The first will have no writes, the second one diff --git a/test/lit/passes/type-ssa_and_merging.wast b/test/lit/passes/type-ssa_and_merging.wast index 40322bfcb..291b37cc2 100644 --- a/test/lit/passes/type-ssa_and_merging.wast +++ b/test/lit/passes/type-ssa_and_merging.wast @@ -1,16 +1,16 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: foreach %s %t wasm-opt --nominal --gufa -Os -all -S -o - | filecheck %s --check-prefix NOP -;; RUN: foreach %s %t wasm-opt --nominal --type-ssa --gufa -Os --type-merging -all -S -o - | filecheck %s --check-prefix YES +;; RUN: foreach %s %t wasm-opt --nominal --closed-world --gufa -Os -all -S -o - | filecheck %s --check-prefix NOP +;; RUN: foreach %s %t wasm-opt --nominal --closed-world --type-ssa --gufa -Os --type-merging -all -S -o - | filecheck %s --check-prefix YES ;; Show that the combination of type-ssa and type-merging can find things that ;; otherwise cannot be optimized. NOP will fail to optimize something that YES ;; can. (module - ;; NOP: (type $A (struct (field (mut i32)))) + ;; NOP: (type $A (struct (field i32))) ;; YES: (type $none_=>_i32 (func (result i32))) - ;; YES: (type $A (struct (field (mut i32)))) + ;; YES: (type $A (struct )) (type $A (struct_subtype (field (mut i32)) data)) ;; NOP: (type $none_=>_i32 (func (result i32))) @@ -40,9 +40,7 @@ ;; YES: (func $main1 (type $none_=>_i32) (result i32) ;; YES-NEXT: (call $get-a-1 - ;; YES-NEXT: (struct.new $A - ;; YES-NEXT: (i32.const 42) - ;; YES-NEXT: ) + ;; YES-NEXT: (struct.new_default $A) ;; YES-NEXT: ) ;; YES-NEXT: (i32.const 42) ;; YES-NEXT: ) @@ -62,9 +60,7 @@ ;; NOP-NEXT: ) ;; YES: (func $main2 (type $none_=>_i32) (result i32) ;; YES-NEXT: (call $get-a-2 - ;; YES-NEXT: (struct.new $A - ;; YES-NEXT: (i32.const 1337) - ;; YES-NEXT: ) + ;; YES-NEXT: (struct.new_default $A) ;; YES-NEXT: ) ;; YES-NEXT: (i32.const 1337) ;; YES-NEXT: ) |