Remove equirecursive typing (#5240)

Equirecursive is no longer standards track and its implementation is extremely
complex. Remove it.

Author: tlively

CHANGELOG.md

@@ -16,7 +16,8 @@ Current Trunk
 -------------
 
 - The isorecursive WasmGC type system (i.e. --hybrid) is now the default to
-  match the spec.
+  match the spec and the old default equirecursive (i.e. --structural) system
+  has been removed.
 
 v111
 ----

src/binaryen-c.cpp

@@ -408,9 +408,6 @@ BinaryenType BinaryenTypeFromHeapType(BinaryenHeapType heapType,
 
 // TypeSystem
 
-BinaryenTypeSystem BinaryenTypeSystemEquirecursive() {
-  return static_cast<BinaryenTypeSystem>(TypeSystem::Equirecursive);
-}
 BinaryenTypeSystem BinaryenTypeSystemNominal() {
   return static_cast<BinaryenTypeSystem>(TypeSystem::Nominal);
 }

src/binaryen-c.h

@@ -192,7 +192,6 @@ BINARYEN_API BinaryenType BinaryenTypeFromHeapType(BinaryenHeapType heapType,
 
 typedef uint32_t BinaryenTypeSystem;
 
-BINARYEN_API BinaryenTypeSystem BinaryenTypeSystemEquirecursive(void);
 BINARYEN_API BinaryenTypeSystem BinaryenTypeSystemNominal(void);
 BINARYEN_API BinaryenTypeSystem BinaryenTypeSystemIsorecursive(void);
 BINARYEN_API BinaryenTypeSystem BinaryenGetTypeSystem(void);

src/ir/module-utils.cpp

@@ -214,24 +214,6 @@ IndexedHeapTypes getOptimizedIndexedHeapTypes(Module& wasm) {
   TypeSystem system = getTypeSystem();
   Counts counts = getHeapTypeCounts(wasm);
 
-  if (system == TypeSystem::Equirecursive) {
-    // Sort by frequency and then original insertion order.
-    std::vector<std::pair<HeapType, size_t>> sorted(counts.begin(),
-                                                    counts.end());
-    std::stable_sort(sorted.begin(), sorted.end(), [&](auto a, auto b) {
-      return a.second > b.second;
-    });
-
-    // Collect the results.
-    IndexedHeapTypes indexedTypes;
-    for (Index i = 0; i < sorted.size(); ++i) {
-      indexedTypes.types.push_back(sorted[i].first);
-    }
-
-    setIndices(indexedTypes);
-    return indexedTypes;
-  }
-
   // Types have to be arranged into topologically ordered recursion groups.
   // Under isorecrsive typing, the topological sort has to take all referenced
   // rec groups into account but under nominal typing it only has to take
@@ -285,9 +267,6 @@ IndexedHeapTypes getOptimizedIndexedHeapTypes(Module& wasm) {
           info.preds.insert(super->getRecGroup());
         }
         break;
-      case TypeSystem::Equirecursive:
-        WASM_UNREACHABLE(
-          "Equirecursive types should already have been handled");
     }
   }
 

src/ir/possible-contents.cpp

@@ -488,12 +488,6 @@ struct InfoCollector
         }
       }
     }
-    if (type.isRef() && getTypeSystem() != TypeSystem::Nominal &&
-        getTypeSystem() != TypeSystem::Isorecursive) {
-      // We need explicit supers in the SubTyping helper class. Without that,
-      // cannot handle refs, and consider them irrelevant.
-      return false;
-    }
     return true;
   }
 
@@ -1582,11 +1576,8 @@ Flower::Flower(Module& wasm) : wasm(wasm) {
   std::cout << "struct phase\n";
 #endif
 
-  if (getTypeSystem() == TypeSystem::Nominal ||
-      getTypeSystem() == TypeSystem::Isorecursive) {
-    subTypes = std::make_unique<SubTypes>(wasm);
-    maxDepths = subTypes->getMaxDepths();
-  }
+  subTypes = std::make_unique<SubTypes>(wasm);
+  maxDepths = subTypes->getMaxDepths();
 
 #ifdef POSSIBLE_CONTENTS_DEBUG
   std::cout << "Link-targets phase\n";

src/ir/subtypes.h

@@ -29,10 +29,6 @@ namespace wasm {
 // This only scans user types, and not basic types like HeapType::eq.
 struct SubTypes {
   SubTypes(const std::vector<HeapType>& types) : types(types) {
-    if (getTypeSystem() != TypeSystem::Nominal &&
-        getTypeSystem() != TypeSystem::Isorecursive) {
-      Fatal() << "SubTypes requires explicit supers";
-    }
     for (auto type : types) {
       note(type);
     }

src/passes/ConstantFieldPropagation.cpp

@@ -187,10 +187,6 @@ struct ConstantFieldPropagation : public Pass {
     if (!module->features.hasGC()) {
       return;
     }
-    if (getTypeSystem() != TypeSystem::Nominal &&
-        getTypeSystem() != TypeSystem::Isorecursive) {
-      Fatal() << "CFP requires nominal/isorecursive typing";
-    }
 
     // Find and analyze all writes inside each function.
     PCVFunctionStructValuesMap functionNewInfos(*module),

src/passes/GUFA.cpp

@@ -110,13 +110,6 @@ struct GUFAOptimizer
       return;
     }
 
-    if (type.isRef() && (getTypeSystem() != TypeSystem::Nominal &&
-                         getTypeSystem() != TypeSystem::Isorecursive)) {
-      // Without type info we can't analyze subtypes, so we cannot infer
-      // anything about refs.
-      return;
-    }
-
     // Ok, this is an interesting location that we might optimize. See what the
     // oracle says is possible there.
     auto contents = getContents(curr);

src/passes/GlobalRefining.cpp

@@ -38,10 +38,6 @@ struct GlobalRefining : public Pass {
     if (!module->features.hasGC()) {
       return;
     }
-    if (getTypeSystem() != TypeSystem::Nominal &&
-        getTypeSystem() != TypeSystem::Isorecursive) {
-      Fatal() << "GlobalRefining requires nominal/isorecursive typing";
-    }
 
     // First, find all the global.sets.
 

src/passes/GlobalStructInference.cpp

@@ -71,10 +71,6 @@ struct GlobalStructInference : public Pass {
     if (!module->features.hasGC()) {
       return;
     }
-    if (getTypeSystem() != TypeSystem::Nominal &&
-        getTypeSystem() != TypeSystem::Isorecursive) {
-      Fatal() << "GlobalStructInference requires nominal/isorecursive typing";
-    }
 
     // First, find all the information we need. We need to know which struct
     // types are created in functions, because we will not be able to optimize

src/passes/GlobalTypeOptimization.cpp

@@ -117,10 +117,6 @@ struct GlobalTypeOptimization : public Pass {
     if (!module->features.hasGC()) {
       return;
     }
-    if (getTypeSystem() != TypeSystem::Nominal &&
-        getTypeSystem() != TypeSystem::Isorecursive) {
-      Fatal() << "GlobalTypeOptimization requires nominal/isorecursive typing";
-    }
 
     // Find and analyze struct operations inside each function.
     StructUtils::FunctionStructValuesMap<FieldInfo> functionNewInfos(*module),

src/passes/Print.cpp

@@ -3034,9 +3034,7 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
     o << '(';
     printMajor(o, "func ");
     printName(curr->name, o);
-    if (currModule && currModule->features.hasGC() &&
-        (getTypeSystem() == TypeSystem::Nominal ||
-         getTypeSystem() == TypeSystem::Isorecursive)) {
+    if (currModule && currModule->features.hasGC()) {
       o << " (type ";
       printHeapType(o, curr->type, currModule) << ')';
     }

src/passes/SignaturePruning.cpp

@@ -52,10 +52,6 @@ struct SignaturePruning : public Pass {
     if (!module->features.hasGC()) {
       return;
     }
-    if (getTypeSystem() != TypeSystem::Nominal &&
-        getTypeSystem() != TypeSystem::Isorecursive) {
-      Fatal() << "SignaturePruning requires nominal/isorecursive typing";
-    }
 
     if (!module->tables.empty()) {
       // When there are tables we must also take their types into account, which

src/passes/SignatureRefining.cpp

@@ -54,10 +54,6 @@ struct SignatureRefining : public Pass {
     if (!module->features.hasGC()) {
       return;
     }
-    if (getTypeSystem() != TypeSystem::Nominal &&
-        getTypeSystem() != TypeSystem::Isorecursive) {
-      Fatal() << "SignatureRefining requires nominal/hybrid typing";
-    }
 
     if (!module->tables.empty()) {
       // When there are tables we must also take their types into account, which

src/passes/pass.cpp

@@ -593,10 +593,7 @@ void PassRunner::addDefaultGlobalOptimizationPrePasses() {
   if (options.optimizeLevel >= 2) {
     addIfNoDWARFIssues("once-reduction");
   }
-  if (wasm->features.hasGC() &&
-      (getTypeSystem() == TypeSystem::Nominal ||
-       getTypeSystem() == TypeSystem::Isorecursive) &&
-      options.optimizeLevel >= 2) {
+  if (wasm->features.hasGC() && options.optimizeLevel >= 2) {
     addIfNoDWARFIssues("type-refining");
     addIfNoDWARFIssues("signature-pruning");
     addIfNoDWARFIssues("signature-refining");

src/tools/tool-options.h

@@ -148,15 +148,6 @@ struct ToolOptions : public Options {
            [](Options* o, const std::string& argument) {
              setTypeSystem(TypeSystem::Nominal);
            })
-      .add("--structural",
-           "",
-           "Force all GC type definitions to be parsed as structural "
-           "(i.e. equirecursive). This is the default.",
-           ToolOptionsCategory,
-           Options::Arguments::Zero,
-           [](Options* o, const std::string& argument) {
-             setTypeSystem(TypeSystem::Equirecursive);
-           })
       .add("--hybrid",
            "",
            "Force all GC type definitions to be parsed using the isorecursive "
@@ -196,9 +187,7 @@ struct ToolOptions : public Options {
   void applyFeatures(Module& module) const {
     module.features.enable(enabledFeatures);
     module.features.disable(disabledFeatures);
-    // Non-default type systems only make sense with GC enabled. TODO: Error on
-    // non-GC equirecursive types as well once we make isorecursive the default
-    // if we don't remove equirecursive types entirely.
+    // Non-default type systems only make sense with GC enabled.
     if (!module.features.hasGC() && getTypeSystem() == TypeSystem::Nominal) {
       Fatal() << "Nominal typing is only allowed when GC is enabled";
     }

src/tools/wasm-fuzz-types.cpp

@@ -517,25 +517,17 @@ int main(int argc, const char* argv[]) {
               Options::Arguments::Zero,
               [&](Options*, const std::string& arg) { verbose = true; });
 
-  TypeSystem system = TypeSystem::Nominal;
+  TypeSystem system = TypeSystem::Isorecursive;
   options.add(
     "--nominal",
     "",
-    "Use the nominal type system (default)",
+    "Use the nominal type system",
     WasmFuzzTypesOption,
     Options::Arguments::Zero,
     [&](Options*, const std::string& arg) { system = TypeSystem::Nominal; });
-  options.add("--structural",
-              "",
-              "Use the equirecursive type system",
-              WasmFuzzTypesOption,
-              Options::Arguments::Zero,
-              [&](Options*, const std::string& arg) {
-                system = TypeSystem::Equirecursive;
-              });
   options.add("--hybrid",
               "",
-              "Use the isorecursive hybrid type system",
+              "Use the isorecursive hybrid type system (default)",
               WasmFuzzTypesOption,
               Options::Arguments::Zero,
               [&](Options*, const std::string& arg) {

src/tools/wasm-reduce.cpp

@@ -1293,9 +1293,10 @@ int main(int argc, const char* argv[]) {
   }
   if (getTypeSystem() == TypeSystem::Nominal) {
     extraFlags += " --nominal";
-  }
-  if (getTypeSystem() == TypeSystem::Isorecursive) {
+  } else if (getTypeSystem() == TypeSystem::Isorecursive) {
     extraFlags += " --hybrid";
+  } else {
+    WASM_UNREACHABLE("unexpected type system");
   }
 
   if (test.size() == 0) {

src/wasm-type.h

@@ -41,9 +41,8 @@
 namespace wasm {
 
 enum class TypeSystem {
-  Equirecursive,
-  Nominal,
   Isorecursive,
+  Nominal,
 };
 
 // This should only ever be called before any Types or HeapTypes have been

src/wasm/wasm-binary.cpp

@@ -232,12 +232,9 @@ void WasmBinaryWriter::writeTypes() {
   // MVP types are structural and do not use recursion groups.
   TypeSystem typeSystem = getTypeSystem();
   if (!wasm->features.hasGC()) {
-    typeSystem = TypeSystem::Equirecursive;
+    typeSystem = TypeSystem::Isorecursive;
   }
   switch (typeSystem) {
-    case TypeSystem::Equirecursive:
-      numGroups = indexedTypes.types.size();
-      break;
     case TypeSystem::Nominal:
       numGroups = 1;
       break;
@@ -2191,9 +2188,6 @@ void WasmBinaryBuilder::readTypes() {
     auto form = getS32LEB();
     if (form == BinaryConsts::EncodedType::Rec) {
       uint32_t groupSize = getU32LEB();
-      if (getTypeSystem() == TypeSystem::Equirecursive) {
-        throwError("Recursion groups not allowed with equirecursive typing");
-      }
       if (groupSize == 0u) {
         // TODO: Support groups of size zero by shrinking the builder.
         throwError("Recursion groups of size zero not supported");