Add bulk-memory-opt feature and ignore call-indirect-overlong (#7139)

LLVM recently split the bulk-memory-opt feature out from bulk-memory,
containing just memory.copy and memory.fill. This change follows that,
making bulk-memory-opt also enabled when all of bulk-memory is enabled.

It also introduces call-indirect-overlong following LLVM, but ignores
it, since Binaryen has always allowed the encoding (i.e. command
line flags enabling or disabling the feature are accepted but
ignored).

Author: dschuff

src/passes/LLVMMemoryCopyFillLowering.cpp

@@ -49,7 +49,7 @@ struct LLVMMemoryCopyFillLowering
   }
 
   void run(Module* module) override {
-    if (!module->features.hasBulkMemory()) {
+    if (!module->features.hasBulkMemoryOpt()) {
       return;
     }
     if (module->features.hasMemory64() || module->features.hasMultiMemory()) {
@@ -108,7 +108,7 @@ struct LLVMMemoryCopyFillLowering
     } else {
       module->removeFunction(memFillFuncName);
     }
-    module->features.disable(FeatureSet::BulkMemory);
+    module->features.setBulkMemoryOpt(false);
   }
 
   void createMemoryCopyFunc(Module* module) {

src/passes/OptimizeInstructions.cpp

@@ -1275,7 +1275,7 @@ struct OptimizeInstructions
     if (curr->type == Type::unreachable) {
       return;
     }
-    assert(getModule()->features.hasBulkMemory());
+    assert(getModule()->features.hasBulkMemoryOpt());
     if (auto* ret = optimizeMemoryCopy(curr)) {
       return replaceCurrent(ret);
     }
@@ -1285,7 +1285,7 @@ struct OptimizeInstructions
     if (curr->type == Type::unreachable) {
       return;
     }
-    assert(getModule()->features.hasBulkMemory());
+    assert(getModule()->features.hasBulkMemoryOpt());
     if (auto* ret = optimizeMemoryFill(curr)) {
       return replaceCurrent(ret);
     }

src/tools/tool-options.h

@@ -82,7 +82,16 @@ struct ToolOptions : public Options {
       .addFeature(FeatureSet::MutableGlobals, "mutable globals")
       .addFeature(FeatureSet::TruncSat, "nontrapping float-to-int operations")
       .addFeature(FeatureSet::SIMD, "SIMD operations and types")
-      .addFeature(FeatureSet::BulkMemory, "bulk memory operations")
+      .addFeature(FeatureSet::BulkMemory,
+                  "bulk memory operations",
+                  FeatureSet(FeatureSet::BulkMemoryOpt))
+      .addFeature(FeatureSet::BulkMemoryOpt,
+                  "memory.copy and memory.fill",
+                  FeatureSet::None,
+                  FeatureSet(FeatureSet::BulkMemory))
+      .addFeature(FeatureSet::CallIndirectOverlong,
+                  "LEB encoding of call-indirect (Ignored for compatibility as "
+                  "it has no effect on Binaryen)")
       .addFeature(FeatureSet::ExceptionHandling,
                   "exception handling operations")
       .addFeature(FeatureSet::TailCall, "tail call operations")
@@ -200,26 +209,28 @@ struct ToolOptions : public Options {
   }
 
   ToolOptions& addFeature(FeatureSet::Feature feature,
-                          const std::string& description) {
+                          const std::string& description,
+                          FeatureSet impliedEnable = FeatureSet::None,
+                          FeatureSet impliedDisable = FeatureSet::None) {
     (*this)
       .add(std::string("--enable-") + FeatureSet::toString(feature),
            "",
            std::string("Enable ") + description,
            ToolOptionsCategory,
            Arguments::Zero,
-           [this, feature](Options*, const std::string&) {
-             enabledFeatures.set(feature, true);
-             disabledFeatures.set(feature, false);
+           [this, feature, impliedEnable](Options*, const std::string&) {
+             enabledFeatures.set(feature | impliedEnable, true);
+             disabledFeatures.set(feature | impliedEnable, false);
            })
 
       .add(std::string("--disable-") + FeatureSet::toString(feature),
            "",
            std::string("Disable ") + description,
            ToolOptionsCategory,
            Arguments::Zero,
-           [this, feature](Options*, const std::string&) {
-             enabledFeatures.set(feature, false);
-             disabledFeatures.set(feature, true);
+           [this, feature, impliedDisable](Options*, const std::string&) {
+             enabledFeatures.set(feature | impliedDisable, false);
+             disabledFeatures.set(feature | impliedDisable, true);
            });
     return *this;
   }

src/wasm-binary.h

@@ -396,6 +396,8 @@ extern const char* MultiMemoryFeature;
 extern const char* TypedContinuationsFeature;
 extern const char* SharedEverythingFeature;
 extern const char* FP16Feature;
+extern const char* BulkMemoryOptFeature;
+extern const char* CallIndirectOverlongFeature;
 
 enum Subsection {
   NameModule = 0,

src/wasm-features.h

@@ -27,6 +27,8 @@ namespace wasm {
 
 struct FeatureSet {
   enum Feature : uint32_t {
+    // These features are intended to those documented in tool-conventions:
+    // https://github.com/WebAssembly/tool-conventions/blob/main/Linking.md#target-features-section
     None = 0,
     Atomics = 1 << 0,
     MutableGlobals = 1 << 1,
@@ -47,11 +49,16 @@ struct FeatureSet {
     TypedContinuations = 1 << 16,
     SharedEverything = 1 << 17,
     FP16 = 1 << 18,
+    BulkMemoryOpt = 1 << 19, // Just the memory.copy and fill operations
+    // This features is a no-op for compatibility. Having it in this list means
+    // that we can automatically generate tool flags that set it, but otherwise
+    // it does nothing. Binaryen always accepts LEB call-indirect encodings.
+    CallIndirectOverlong = 1 << 20,
     MVP = None,
     // Keep in sync with llvm default features:
     // https://github.com/llvm/llvm-project/blob/c7576cb89d6c95f03968076e902d3adfd1996577/clang/lib/Basic/Targets/WebAssembly.cpp#L150-L153
     Default = SignExt | MutableGlobals,
-    All = (1 << 19) - 1,
+    All = (1 << 21) - 1,
   };
 
   static std::string toString(Feature f) {
@@ -94,6 +101,10 @@ struct FeatureSet {
         return "shared-everything";
       case FP16:
         return "fp16";
+      case BulkMemoryOpt:
+        return "bulk-memory-opt";
+      case CallIndirectOverlong:
+        return "call-indirect-overlong";
       default:
         WASM_UNREACHABLE("unexpected feature");
     }
@@ -145,6 +156,11 @@ struct FeatureSet {
     return (features & SharedEverything) != 0;
   }
   bool hasFP16() const { return (features & FP16) != 0; }
+  bool hasBulkMemoryOpt() const {
+    bool has = (features & BulkMemoryOpt) != 0;
+    assert(has || !hasBulkMemory());
+    return has;
+  }
   bool hasAll() const { return (features & All) != 0; }
 
   void set(FeatureSet f, bool v = true) {
@@ -169,6 +185,7 @@ struct FeatureSet {
   void setTypedContinuations(bool v = true) { set(TypedContinuations, v); }
   void setSharedEverything(bool v = true) { set(SharedEverything, v); }
   void setFP16(bool v = true) { set(FP16, v); }
+  void setBulkMemoryOpt(bool v = true) { set(BulkMemoryOpt, v); }
   void setMVP() { features = MVP; }
   void setAll() { features = All; }
 

src/wasm/wasm-binary.cpp

@@ -1354,6 +1354,10 @@ void WasmBinaryWriter::writeFeaturesSection() {
         return BinaryConsts::CustomSections::SharedEverythingFeature;
       case FeatureSet::FP16:
         return BinaryConsts::CustomSections::FP16Feature;
+      case FeatureSet::BulkMemoryOpt:
+        return BinaryConsts::CustomSections::BulkMemoryOptFeature;
+      case FeatureSet::CallIndirectOverlong:
+        return BinaryConsts::CustomSections::CallIndirectOverlongFeature;
       case FeatureSet::None:
       case FeatureSet::Default:
       case FeatureSet::All:
@@ -4790,6 +4794,12 @@ void WasmBinaryReader::readFeatures(size_t payloadLen) {
       feature = FeatureSet::Atomics;
     } else if (name == BinaryConsts::CustomSections::BulkMemoryFeature) {
       feature = FeatureSet::BulkMemory;
+      if (used) {
+        // For backward compatibility, enable this dependent feature.
+        feature |= FeatureSet::BulkMemoryOpt;
+      }
+    } else if (name == BinaryConsts::CustomSections::BulkMemoryOptFeature) {
+      feature = FeatureSet::BulkMemoryOpt;
     } else if (name == BinaryConsts::CustomSections::ExceptionHandlingFeature) {
       feature = FeatureSet::ExceptionHandling;
     } else if (name == BinaryConsts::CustomSections::MutableGlobalsFeature) {

src/wasm/wasm-validator.cpp

@@ -1526,10 +1526,10 @@ void FunctionValidator::visitDataDrop(DataDrop* curr) {
 }
 
 void FunctionValidator::visitMemoryCopy(MemoryCopy* curr) {
-  shouldBeTrue(
-    getModule()->features.hasBulkMemory(),
-    curr,
-    "Bulk memory operations require bulk memory [--enable-bulk-memory]");
+  shouldBeTrue(getModule()->features.hasBulkMemoryOpt(),
+               curr,
+               "memory.copy operations require bulk memory operations "
+               "[--enable-bulk-memory-opt]");
   shouldBeEqualOrFirstIsUnreachable(
     curr->type, Type(Type::none), curr, "memory.copy must have type none");
   auto* destMemory = getModule()->getMemoryOrNull(curr->destMemory);
@@ -1561,9 +1561,9 @@ void FunctionValidator::visitMemoryCopy(MemoryCopy* curr) {
 void FunctionValidator::visitMemoryFill(MemoryFill* curr) {
   auto* memory = getModule()->getMemoryOrNull(curr->memory);
   shouldBeTrue(
-    getModule()->features.hasBulkMemory(),
+    getModule()->features.hasBulkMemoryOpt(),
     curr,
-    "Bulk memory operations require bulk memory [--enable-bulk-memory]");
+    "memory.fill operations require bulk memory [--enable-bulk-memory-opt]");
   shouldBeEqualOrFirstIsUnreachable(
     curr->type, Type(Type::none), curr, "memory.fill must have type none");
   shouldBeEqualOrFirstIsUnreachable(

src/wasm/wasm.cpp

@@ -57,6 +57,8 @@ const char* MultiMemoryFeature = "multimemory";
 const char* TypedContinuationsFeature = "typed-continuations";
 const char* SharedEverythingFeature = "shared-everything";
 const char* FP16Feature = "fp16";
+const char* BulkMemoryOptFeature = "bulk-memory-opt";
+const char* CallIndirectOverlongFeature = "call-indirect-overlong";
 } // namespace CustomSections
 } // namespace BinaryConsts
 

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: 524287
+Features.All: 2097151
 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: 524287
+BinaryenFeatureAll: 2097151
 (f32.neg
  (f32.const -33.61199951171875)
 )