diff --git a/src/ir/type-updating.h b/src/ir/type-updating.h index 3333b288461..aa0106d09df 100644 --- a/src/ir/type-updating.h +++ b/src/ir/type-updating.h @@ -580,11 +580,11 @@ Expression* fixLocalGet(LocalGet* get, Module& wasm); // Applies new types of parameters to a function. This does all the necessary // changes aside from altering the function type, which the caller is expected // to do after we run (the caller might simply change the type, but in other -// cases the caller might be rewriting the types and need to preserve their -// identity in terms of nominal typing, so we don't change the type here). The -// specific things this function does are to update the types of local.get/tee -// operations, refinalize, etc., basically all operations necessary to ensure -// validation with the new types. +// cases the caller might be rewriting the types and need to preserve their +// identities, so we don't change the type here). The specific things this +// function does are to update the types of local.get/tee operations, +// refinalize, etc., basically all operations necessary to ensure validation +// with the new types. // // While doing so, we can either update or not update the types of local.get and // local.tee operations. (We do not update them here if we'll be doing an update diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index a2231165294..3af16ee9f56 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -5726,7 +5726,9 @@ struct OptimizeInstructions // curr, and so they must be compatible to allow for a proper new // type after the transformation. // - // At minimum an LUB is required, as shown here: + // The children must have a shared supertype. For example, we cannot + // fold out the `drop` here because there would be no valid result + // type for the if afterward: // // (if // (condition) @@ -5734,8 +5736,10 @@ struct OptimizeInstructions // (drop (f64.const 2.0)) // ) // - // However, that may not be enough, as with nominal types we can - // have things like this: + // However, having a shared supertype may not be enough. If $A and + // $B have a shared supertype, but that supertype does not have a + // field at index 1, then we cannot fold the duplicated struct.get + // here: // // (if // (condition) @@ -5743,48 +5747,26 @@ struct OptimizeInstructions // (struct.get $B 1 (..)) // ) // - // It is possible that the LUB of $A and $B does not contain field - // "1". With structural types this specific problem is not possible, - // and it appears to be the case that with the GC MVP there is no - // instruction that poses a problem, but in principle it can happen - // there as well, if we add an instruction that returns the number - // of fields in a type, for example. For that reason, and to avoid - // a difference between structural and nominal typing here, disallow - // subtyping in both. (Note: In that example, the problem only - // happens because the type is not part of the struct.get - we infer - // it from the reference. That is why after hoisting the struct.get - // out, and computing a new type for the if that is now the child of - // the single struct.get, we get a struct.get of a supertype. So in - // principle we could fix this by modifying the IR as well, but the - // problem is more general, so avoid that.) + // (Note: In that example, the problem only happens because the type + // is not part of the struct.get - we infer it from the reference. + // That is why after hoisting the struct.get out, and computing a + // new type for the if that is now the child of the single + // struct.get, we get a struct.get of a supertype. So in principle + // we could fix this by modifying the IR as well, but the problem is + // more general, so avoid that.) + // + // For now, only support the case where the types of the children + // are the same. + // + // TODO: We could analyze whether the LUB of the children types + // would satisfy the constraints imposed by the shared parent + // instruction. This would require a version of ChildTyper that + // allows for generalizing type annotations. ChildIterator ifFalseChildren(curr->ifFalse); auto* ifTrueChild = *ifTrueChildren.begin(); auto* ifFalseChild = *ifFalseChildren.begin(); bool validTypes = ifTrueChild->type == ifFalseChild->type; - // In addition, after we move code outside of curr then we need to - // not change unreachability - if we did, we'd need to propagate - // that further, and we leave such work to DCE and Vacuum anyhow. - // This can happen in something like this for example, where the - // outer type changes from i32 to unreachable if we move the - // returns outside: - // - // (if (result i32) - // (local.get $x) - // (return - // (local.get $y) - // ) - // (return - // (local.get $z) - // ) - // ) - assert(curr->ifTrue->type == curr->ifFalse->type); - auto newOuterType = curr->ifTrue->type; - if ((newOuterType == Type::unreachable) != - (curr->type == Type::unreachable)) { - validTypes = false; - } - // If the expression we are about to move outside has side effects, // then we cannot do so in general with a select: we'd be reducing // the amount of the effects as well as moving them. For an if, diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp index a867dffa255..d1f1a14456d 100644 --- a/src/passes/SignaturePruning.cpp +++ b/src/passes/SignaturePruning.cpp @@ -279,13 +279,12 @@ struct SignaturePruning : public Pass { // Create a new signature. When the TypeRewriter operates below it will // modify the existing heap type in place to change its signature to this - // one (which preserves identity, that is, even if after pruning the new - // signature is structurally identical to another one, it will remain - // nominally different from those). + // one. TypeRewriter will also ensure that distinct types remain + // disctinct, even if they have the same signature after optimization. newSignatures[type] = Signature(Type(newParams), sig.results); // removeParameters() updates the type as it goes, but in this pass we - // need the type to match the other locations, nominally. That is, we need + // need the type to be updated in all locations at once. That is, we need // all the functions of a particular type to still have the same type // after this operation, and that must be the exact same type at the // relevant call_refs and so forth. The TypeRewriter below will do the diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 56a07ca2d44..e4c0d6cf9bb 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -569,7 +569,7 @@ void PassRegistry::registerPasses() { "merge types to their supertypes where possible", createTypeMergingPass); registerPass("type-ssa", - "create new nominal types to help other optimizations", + "create new types to help other optimizations", createTypeSSAPass); registerPass("type-unfinalizing", "mark all types as non-final (open)", diff --git a/src/wasm-type.h b/src/wasm-type.h index 98aacb074b2..fcd99063450 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -139,21 +139,13 @@ class HeapType { // Choose an arbitrary heap type as the default. constexpr HeapType() : HeapType(func) {} - // Construct a HeapType referring to the single canonical HeapType for the - // given signature. In nominal mode, this is the first HeapType created with - // this signature. + // Construct the single canonical HeapType for the given signature, struct, + // array, or continuation. HeapType(Signature signature); - - HeapType(Continuation cont); - - // Create a HeapType with the given structure. In equirecursive mode, this may - // be the same as a previous HeapType created with the same contents. In - // nominal mode, this will be a fresh type distinct from all previously - // created HeapTypes. - // TODO: make these explicit to differentiate them. HeapType(const Struct& struct_); HeapType(Struct&& struct_); HeapType(Array array); + HeapType(Continuation cont); HeapTypeKind getKind() const; @@ -193,7 +185,7 @@ class HeapType { Array getArray() const; // If there is a nontrivial (i.e. non-basic, one that was declared by the - // module) nominal supertype, return it, else an empty optional. + // module) supertype, return it, else an empty optional. std::optional getDeclaredSuperType() const; // As |getDeclaredSuperType|, but also handles basic types, that is, if the @@ -206,8 +198,8 @@ class HeapType { std::optional getDescribedType() const; DescriptorChain getDescriptorChain() const; - // Return the depth of this heap type in the nominal type hierarchy, i.e. the - // number of supertypes in its supertype chain. + // Return the depth of this heap type in the type hierarchy, i.e. the number + // of supertypes in its supertype chain. size_t getDepth() const; // Get the bottom heap type for this heap type's hierarchy. @@ -898,10 +890,7 @@ struct TypeBuilder { }; // Returns all of the newly constructed heap types. May only be called once - // all of the heap types have been initialized with `setHeapType`. In nominal - // mode, all of the constructed HeapTypes will be fresh and distinct. In - // nominal mode, will also produce a fatal error if the declared subtype - // relationships are not valid. + // all of the heap types have been initialized with `setHeapType`. BuildResult build(); // Utility for ergonomically using operator[] instead of explicit setHeapType diff --git a/test/lit/help/wasm-metadce.test b/test/lit/help/wasm-metadce.test index a8906472a32..32faa584eba 100644 --- a/test/lit/help/wasm-metadce.test +++ b/test/lit/help/wasm-metadce.test @@ -557,8 +557,8 @@ ;; CHECK-NEXT: type fields where possible ;; CHECK-NEXT: (using GUFA) ;; CHECK-NEXT: -;; CHECK-NEXT: --type-ssa create new nominal types to help -;; CHECK-NEXT: other optimizations +;; CHECK-NEXT: --type-ssa create new types to help other +;; CHECK-NEXT: optimizations ;; CHECK-NEXT: ;; CHECK-NEXT: --type-unfinalizing mark all types as non-final ;; CHECK-NEXT: (open) diff --git a/test/lit/help/wasm-opt.test b/test/lit/help/wasm-opt.test index 81ab778afe9..e84623f6cca 100644 --- a/test/lit/help/wasm-opt.test +++ b/test/lit/help/wasm-opt.test @@ -589,8 +589,8 @@ ;; CHECK-NEXT: type fields where possible ;; CHECK-NEXT: (using GUFA) ;; CHECK-NEXT: -;; CHECK-NEXT: --type-ssa create new nominal types to help -;; CHECK-NEXT: other optimizations +;; CHECK-NEXT: --type-ssa create new types to help other +;; CHECK-NEXT: optimizations ;; CHECK-NEXT: ;; CHECK-NEXT: --type-unfinalizing mark all types as non-final ;; CHECK-NEXT: (open) diff --git a/test/lit/help/wasm2js.test b/test/lit/help/wasm2js.test index f2102f89f7d..d1478383caa 100644 --- a/test/lit/help/wasm2js.test +++ b/test/lit/help/wasm2js.test @@ -521,8 +521,8 @@ ;; CHECK-NEXT: type fields where possible ;; CHECK-NEXT: (using GUFA) ;; CHECK-NEXT: -;; CHECK-NEXT: --type-ssa create new nominal types to help -;; CHECK-NEXT: other optimizations +;; CHECK-NEXT: --type-ssa create new types to help other +;; CHECK-NEXT: optimizations ;; CHECK-NEXT: ;; CHECK-NEXT: --type-unfinalizing mark all types as non-final ;; CHECK-NEXT: (open) diff --git a/test/lit/parse-bad-nominal-types.wast b/test/lit/parse-missing-supertype.wast similarity index 82% rename from test/lit/parse-bad-nominal-types.wast rename to test/lit/parse-missing-supertype.wast index 1801f13aa19..d561b7fbbc0 100644 --- a/test/lit/parse-bad-nominal-types.wast +++ b/test/lit/parse-missing-supertype.wast @@ -1,4 +1,4 @@ -;; Test that incorrect nominal types result in the expected parse errors +;; Test that missing supertypes result in the expected parse errors ;; RUN: foreach %s %t not wasm-opt -all 2>&1 | filecheck %s diff --git a/test/lit/passes/gto_and_cfp_in_O.wast b/test/lit/passes/gto_and_cfp_in_O.wast index 783da3a1982..7b1ecb74a72 100644 --- a/test/lit/passes/gto_and_cfp_in_O.wast +++ b/test/lit/passes/gto_and_cfp_in_O.wast @@ -3,9 +3,8 @@ ;; RUN: foreach %s %t wasm-opt -O -all --closed-world -S -o - | filecheck %s ;; RUN: foreach %s %t wasm-opt -O -all -S -o - | filecheck %s --check-prefix OPEN_WORLD -;; Test that -O, with nominal typing + GC enabled, will run global type -;; optimization in conjunction with constant field propagation etc. But, in an -;; open world we do not run them. +;; Test that -O with GC enabled will run global type optimization in conjunction +;; with constant field propagation etc. in closed world mode only. (module ;; OPEN_WORLD: (type $struct (sub (struct (field (mut funcref)) (field (mut i32))))) diff --git a/test/lit/passes/merge-similar-functions_types.wast b/test/lit/passes/merge-similar-functions_types.wast index aaf36d34595..735321db53a 100644 --- a/test/lit/passes/merge-similar-functions_types.wast +++ b/test/lit/passes/merge-similar-functions_types.wast @@ -4,24 +4,37 @@ ;; Calls to functions $2 and $3 are the only differences between the contents ;; of $0 and $1, so we want to merge them and pass ref.funcs of $2 and $3. -;; However, their nominal types differ, so in nominal typing we cannot do so. +;; However, their types differ, so we cannot do so. (module - ;; CHECK: (type $type$0 (sub (func))) - (type $type$0 (sub (func))) - (type $type$1 (sub (func))) - (type $type$2 (sub (func))) - (type $type$3 (sub (func (param f32) (result f32)))) - (type $type$4 (sub (func (param f64) (result f64)))) - ;; CHECK: (type $1 (func (param (ref $type$0)))) + (rec + ;; CHECK: (type $0 (func)) - ;; CHECK: (elem declare func $2 $3) - - ;; CHECK: (func $0 (type $type$0) - ;; CHECK-NEXT: (return_call $byn$mgfn-shared$0 - ;; CHECK-NEXT: (ref.func $2) - ;; CHECK-NEXT: ) + ;; CHECK: (rec + ;; CHECK-NEXT: (type $type$1 (sub (func))) + (type $type$1 (sub (func))) + ;; CHECK: (type $type$2 (sub (func))) + (type $type$2 (sub (func))) + ) + ;; CHECK: (func $0 (type $0) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (call $2) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) - (func $0 (type $type$0) + (func $0 (nop) (nop) (nop) @@ -40,12 +53,26 @@ (nop) (nop) ) - ;; CHECK: (func $1 (type $type$0) - ;; CHECK-NEXT: (return_call $byn$mgfn-shared$0 - ;; CHECK-NEXT: (ref.func $3) - ;; CHECK-NEXT: ) + ;; CHECK: (func $1 (type $0) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (call $3) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) - (func $1 (type $type$0) + (func $1 (nop) (nop) (nop) @@ -64,7 +91,7 @@ (nop) (nop) ) - ;; CHECK: (func $2 (type $type$0) + ;; CHECK: (func $2 (type $type$1) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i32.const 17) ;; CHECK-NEXT: ) @@ -74,7 +101,7 @@ (i32.const 17) ) ) - ;; CHECK: (func $3 (type $type$0) + ;; CHECK: (func $3 (type $type$2) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i32.const 999) ;; CHECK-NEXT: ) @@ -86,37 +113,11 @@ ) ) -;; CHECK: (func $byn$mgfn-shared$0 (type $1) (param $0 (ref $type$0)) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (call_ref $type$0 -;; CHECK-NEXT: (local.get $0) -;; CHECK-NEXT: ) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: ) (module - ;; As above, but now the nominal types do match, so we can optimize in all - ;; modes. - ;; CHECK: (type $type$0 (sub (func))) - (type $type$0 (sub (func))) - (type $type$1 (sub (func))) - (type $type$3 (sub (func (param f32) (result f32)))) - (type $type$4 (sub (func (param f64) (result f64)))) + ;; As above, but now the types are the same, so we can optimize. + ;; CHECK: (type $0 (func)) - ;; CHECK: (type $1 (func (param (ref $type$0)))) + ;; CHECK: (type $1 (func (param (ref $0)))) ;; CHECK: (global $global$0 (mut i32) (i32.const 10)) (global $global$0 (mut i32) (i32.const 10)) @@ -125,12 +126,12 @@ ;; CHECK: (elem declare func $2 $3) - ;; CHECK: (func $0 (type $type$0) + ;; CHECK: (func $0 (type $0) ;; CHECK-NEXT: (return_call $byn$mgfn-shared$0 ;; CHECK-NEXT: (ref.func $2) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $0 (type $type$0) + (func $0 (nop) (nop) (nop) @@ -149,12 +150,12 @@ (nop) (nop) ) - ;; CHECK: (func $1 (type $type$0) + ;; CHECK: (func $1 (type $0) ;; CHECK-NEXT: (return_call $byn$mgfn-shared$0 ;; CHECK-NEXT: (ref.func $3) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $1 (type $type$0) + (func $1 (nop) (nop) (nop) @@ -173,28 +174,28 @@ (nop) (nop) ) - ;; CHECK: (func $2 (type $type$0) + ;; CHECK: (func $2 (type $0) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i32.const 17) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $2 (type $type$1) + (func $2 (drop (i32.const 17) ) ) - ;; CHECK: (func $3 (type $type$0) + ;; CHECK: (func $3 (type $0) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i32.const 999) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $3 (type $type$1) + (func $3 (drop (i32.const 999) ) ) ) -;; CHECK: (func $byn$mgfn-shared$0 (type $1) (param $0 (ref $type$0)) +;; CHECK: (func $byn$mgfn-shared$0 (type $1) (param $0 (ref $0)) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (nop) @@ -203,7 +204,7 @@ ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (nop) -;; CHECK-NEXT: (call_ref $type$0 +;; CHECK-NEXT: (call_ref $0 ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (nop) diff --git a/test/lit/passes/optimize-instructions-call_ref-roundtrip.wast b/test/lit/passes/optimize-instructions-call_ref-roundtrip.wast index c3e51d04984..4cba4af9399 100644 --- a/test/lit/passes/optimize-instructions-call_ref-roundtrip.wast +++ b/test/lit/passes/optimize-instructions-call_ref-roundtrip.wast @@ -1,14 +1,13 @@ ;; 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. +;; roundtrip to see the effects on heap types in the binary format. ;; RUN: wasm-opt %s --optimize-instructions --roundtrip --all-features -S -o - | filecheck %s -;; roundtrip to see the effects on heap types in the binary format, specifically -;; regarding nominal heap types (module - ;; Create multiple different signature types, all identical structurally but - ;; distinct nominally. The three tables will use different ones, and the - ;; emitted call_indirects should use the corresponding ones. + ;; Create multiple different function types, all with the same signature. The + ;; three tables will use different types, and the emitted call_indirects should + ;; use the corresponding types. (rec ;; CHECK: (rec diff --git a/test/lit/passes/optimize-instructions-gc.wast b/test/lit/passes/optimize-instructions-gc.wast index 8d413df37aa..8902a90d555 100644 --- a/test/lit/passes/optimize-instructions-gc.wast +++ b/test/lit/passes/optimize-instructions-gc.wast @@ -1108,14 +1108,9 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $hoist-LUB-danger (param $x i32) (param $b (ref $B)) (param $c (ref $C)) (result i32) - ;; In nominal typing, if we hoist the struct.get out of the if, then the if - ;; will have a new type, $A, but $A does not have field "1" which would be an - ;; error. We disallow subtyping for this reason. - ;; - ;; We also disallow subtyping in structural typing, even though atm there - ;; might not be a concrete risk there: future instructions might introduce - ;; such things, and it reduces the complexity of having differences with - ;; nominal typing. + ;; If we hoist the struct.get out of the if, then the if will have a new + ;; type, $A, but $A does not have field 1, so that would be an error. We + ;; disallow hoisting when the children have different types for this reason. (if (result i32) (local.get $x) (then diff --git a/test/lit/passes/signature-pruning.wast b/test/lit/passes/signature-pruning.wast index a5791729f9a..30ddd25f1ab 100644 --- a/test/lit/passes/signature-pruning.wast +++ b/test/lit/passes/signature-pruning.wast @@ -608,8 +608,7 @@ ;; Two functions with two different types have an unused parameter. After ;; removing the parameter from each, they both have no parameters. They should -;; *not* have the same type, however: the type should be different nominally -;; even though after the pruning they are identical structurally. +;; *not* have the same type, however, even though they have the same signature. (module (rec ;; CHECK: (rec