[Strings] Add a string-builtins feature, and lift/lower automatically when enabled

README.md

@@ -201,7 +201,9 @@ There are a few differences between Binaryen IR and the WebAssembly language:
      stringref operations, before any default optimization passes. Those
      stringref operations can then be optimized (e.g., a concat of constants
      turns into a concatenated constant). When we are about to finish running
-     default optimizations, we lower stringref back into string builtins.
+     default optimizations, we lower stringref back into string builtins. (Note:
+     reference types and GC must also be enabled, as imported string operations
+     depend on GC arrays.)
 
 As a result, you might notice that round-trip conversions (wasm => Binaryen IR
 => wasm) change code a little in some corner cases.

src/passes/pass.cpp

@@ -727,8 +727,11 @@ void PassRunner::addDefaultGlobalOptimizationPrePasses(Ordering ordering) {
   // If we are optimizing string builtins then we lift at the very start of the
   // optimization pipeline, not just at the beginning here, but only when we are
   // ordered before other bundles of passes.
-  if (wasm->features.hasStringBuiltins() && options.optimizeLevel >= 2 &&
-      ordering.first) {
+  //
+  // We check for GC for symmetry with the lowering pass, see comment in
+  // addDefaultGlobalOptimizationPostPasses() below.
+  if (wasm->features.hasStringBuiltins() && wasm->features.hasGC() &&
+      options.optimizeLevel >= 2 && ordering.first) {
     addIfNoDWARFIssues("string-lifting");
   }
   // Removing duplicate functions is fast and saves work later.
@@ -805,9 +808,14 @@ void PassRunner::addDefaultGlobalOptimizationPostPasses(Ordering ordering) {
   // Lower away strings at the very very end. We do this before
   // remove-unused-module-elements so we don't add unused imports, and also
   // before reorder-globals, which will sort the new globals.
-  if (wasm->features.hasStringBuiltins() && options.optimizeLevel >= 2 &&
-      ordering.last) {
-    addIfNoDWARFIssues("string-lowering");
+  //
+  // Note we also test for GC here, as the pass adds imports that use GC arrays
+  // (and externref). Those imports may be unused, but they exist until
+  // remove-unused-module-elements cleans them up, which would cause an error in
+  // between.
+  if (wasm->features.hasStringBuiltins() && wasm->features.hasGC() &&
+      options.optimizeLevel >= 2 && ordering.last) {
+    addIfNoDWARFIssues("string-lowering-magic-imports");
   }
 
   addIfNoDWARFIssues("remove-unused-module-elements");

src/wasm/wasm-binary.cpp

@@ -5195,6 +5195,8 @@ void WasmBinaryReader::readFeatures(size_t payloadLen) {
       feature = FeatureSet::FP16;
     } else if (name == BinaryConsts::CustomSections::CustomDescriptorsFeature) {
       feature = FeatureSet::CustomDescriptors;
+    } else if (name == BinaryConsts::CustomSections::StringBuiltinsFeature) {
+      feature = FeatureSet::StringBuiltins;
     } else {
       // Silently ignore unknown features (this may be and old binaryen running
       // on a new wasm).

test/binaryen.js/kitchen-sink.js.txt

@@ -33,7 +33,7 @@ Features.RelaxedSIMD: 4096
 Features.ExtendedConst: 8192
 Features.Strings: 16384
 Features.MultiMemory: 32768
-Features.All: 4194303
+Features.All: 8388607
 InvalidId: 0
 BlockId: 1
 IfId: 2

test/example/c-api-kitchen-sink.txt

@@ -47,7 +47,7 @@ BinaryenFeatureMemory64: 2048
 BinaryenFeatureRelaxedSIMD: 4096
 BinaryenFeatureExtendedConst: 8192
 BinaryenFeatureStrings: 16384
-BinaryenFeatureAll: 4194303
+BinaryenFeatureAll: 8388607
 (f32.neg
  (f32.const -33.61199951171875)
 )