[NFC] Remove outdated mentions of nominal types (#8142)

Fix comments and tests mentioning the experimental nominal type system
that existed before GC was standardized. As a drive-by, remove some
adjacent dead code in OptimizeInstructions that has not been relevant
since we updated If finalization to propage unreachability.

Author: tlively

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

src/passes/OptimizeInstructions.cpp

@@ -5726,65 +5726,47 @@ 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)
             //    (drop (i32.const 1))
             //    (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)
             //    (struct.get $A 1 (..))
             //    (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,

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

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)",

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<HeapType> getDeclaredSuperType() const;
 
   // As |getDeclaredSuperType|, but also handles basic types, that is, if the
@@ -206,8 +198,8 @@ class HeapType {
   std::optional<HeapType> 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

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)

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)

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)

test/lit/parse-bad-nominal-types.wast test/lit/parse-missing-supertype.wasttest/lit/parse-bad-nominal-types.wast renamed to 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
 

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)))))