Feature options (#1797)

Add feature flags and struct interface. Default feature set has all feature enabled.

Author: tlively

auto_update_tests.py

@@ -34,7 +34,7 @@ def update_asm_js_tests():
     if asm.endswith('.asm.js'):
       for precise in [0, 1, 2]:
         for opts in [1, 0]:
-          cmd = ASM2WASM + [os.path.join('test', asm), '--enable-threads']
+          cmd = ASM2WASM + [os.path.join('test', asm)]
           wasm = asm.replace('.asm.js', '.fromasm')
           if not precise:
             cmd += ['--trap-mode=allow', '--ignore-implicit-traps']

src/binaryen-c.cpp

@@ -1936,7 +1936,7 @@ int BinaryenModuleValidate(BinaryenModuleRef module) {
 
   Module* wasm = (Module*)module;
   // TODO add feature selection support to C API
-  FeatureSet features = Feature::Atomics;
+  FeatureSet features = FeatureSet::All;
   return WasmValidator().validate(*wasm, features) ? 1 : 0;
 }
 

src/pass.h

@@ -63,7 +63,7 @@ struct PassOptions {
   int shrinkLevel = 0;   // 0, 1, 2 correspond to -O0, -Os, -Oz
   bool ignoreImplicitTraps = false; // optimize assuming things like div by 0, bad load/store, will not trap
   bool debugInfo = false; // whether to try to preserve debug info through, which are special calls
-  FeatureSet features = Feature::MVP; // Which wasm features to accept, and be allowed to use
+  FeatureSet features = FeatureSet::All; // Which wasm features to accept, and be allowed to use
 
   void setDefaultOptimizationOptions() {
     // -Os is our default

src/passes/Print.cpp

@@ -1184,7 +1184,7 @@ Pass* createPrintStackIRPass() {
 
 std::ostream& WasmPrinter::printModule(Module* module, std::ostream& o) {
   PassRunner passRunner(module);
-  passRunner.setFeatures(Feature::All);
+  passRunner.setFeatures(FeatureSet::All);
   passRunner.setIsNested(true);
   passRunner.add<Printer>(&o);
   passRunner.run();

src/tools/asm2wasm.cpp

@@ -115,9 +115,6 @@ int main(int argc, const char *argv[]) {
       .add("--emit-text", "-S", "Emit text instead of binary for the output file",
            Options::Arguments::Zero,
            [&](Options *o, const std::string& argument) { emitBinary = false; })
-      .add("--enable-threads", "-a", "Enable the Atomics wasm feature",
-           Options::Arguments::Zero,
-           [&](Options *o, const std::string& argument) { options.passOptions.features |= Feature::Atomics; })
       .add_positional("INFILE", Options::Arguments::One,
                       [](Options *o, const std::string& argument) {
                         o->extra["infile"] = argument;
@@ -188,7 +185,7 @@ int main(int argc, const char *argv[]) {
   if (memInit != options.extra.end()) {
     if (options.runningDefaultOptimizationPasses()) {
       PassRunner runner(&wasm);
-      runner.setFeatures(options.features);
+      runner.setFeatures(options.getFeatures());
       runner.add("memory-packing");
       runner.run();
     }

src/tools/feature-options.h

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2018 WebAssembly Community Group participants
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "support/command-line.h"
+#include "pass.h"
+
+//
+// Shared optimization options for commandline tools
+//
+
+namespace wasm {
+
+struct FeatureOptions : public Options {
+  PassOptions passOptions;
+
+  FeatureOptions(const std::string& command, const std::string& description)
+      : Options(command, description) {
+    (*this)
+        .add("--mvp-features", "-mvp", "Disable all non-MVP features (default)",
+             Options::Arguments::Zero,
+             [this](Options *o, const std::string& arguments) {
+               passOptions.features = FeatureSet::MVP;
+             })
+        .add("--all-features", "-all", "Enable all features",
+             Options::Arguments::Zero,
+             [this](Options *o, const std::string& arguments) {
+               passOptions.features = FeatureSet::All;
+             })
+        .add("--enable-threads", "", "Enable atomic operations",
+             Options::Arguments::Zero,
+             [this](Options *o, const std::string& arguments) {
+               passOptions.features.setAtomics();
+             })
+        .add("--disable-threads", "", "Disable atomic operations",
+             Options::Arguments::Zero,
+             [this](Options *o, const std::string& arguments) {
+               passOptions.features.setAtomics(false);
+             })
+        .add("--enable-mutable-globals", "", "Enable mutable globals",
+             Options::Arguments::Zero,
+             [this](Options *o, const std::string& arguments) {
+               passOptions.features.setMutableGlobals();
+             })
+        .add("--disable-mutable-globals", "", "Disable mutable globals",
+             Options::Arguments::Zero,
+             [this](Options *o, const std::string& arguments) {
+               passOptions.features.setMutableGlobals(false);
+             })
+        .add("--enable-nontrapping-float-to-int", "",
+             "Enable nontrapping float-to-int operations",
+             Options::Arguments::Zero,
+             [this](Options *o, const std::string& arguments) {
+               passOptions.features.setTruncSat();
+             })
+        .add("--disable-nontrapping-float-to-int", "",
+             "Disable nontrapping float-to-int operations",
+             Options::Arguments::Zero,
+             [this](Options *o, const std::string& arguments) {
+               passOptions.features.setTruncSat(false);
+             });
+  }
+
+  FeatureSet getFeatures() const {
+    return passOptions.features;
+  }
+};
+
+} // namespace wasm

src/tools/fuzzing.h

@@ -123,8 +123,8 @@ class TranslateToFuzzReader {
     std::cout << "shrink level: " << options.passOptions.shrinkLevel << '\n';
   }
 
-  void build(bool initEmitAtomics = true) {
-    emitAtomics = initEmitAtomics;
+  void build(FeatureSet features_) {
+    features = features_;
     setupMemory();
     setupTable();
     setupGlobals();
@@ -179,8 +179,8 @@ class TranslateToFuzzReader {
   // cross-VM comparisons harder)
   static const bool DE_NAN = true;
 
-  // Whether to emit atomics
-  bool emitAtomics = true;
+  // Features allowed to be emitted
+  FeatureSet features = FeatureSet::All;
 
   // Whether to emit atomic waits (which in single-threaded mode, may hang...)
   static const bool ATOMIC_WAITS = false;
@@ -1118,7 +1118,7 @@ class TranslateToFuzzReader {
   Expression* makeLoad(Type type) {
     auto* ret = makeNonAtomicLoad(type);
     if (type != i32 && type != i64) return ret;
-    if (!emitAtomics || oneIn(2)) return ret;
+    if (!features.hasAtomics() || oneIn(2)) return ret;
     // make it atomic
     wasm.memory.shared = true;
     ret->isAtomic = true;
@@ -1181,7 +1181,7 @@ class TranslateToFuzzReader {
   Store* makeStore(Type type) {
     auto* ret = makeNonAtomicStore(type);
     if (ret->value->type != i32 && ret->value->type != i64) return ret;
-    if (!emitAtomics || oneIn(2)) return ret;
+    if (!features.hasAtomics() || oneIn(2)) return ret;
     // make it atomic
     wasm.memory.shared = true;
     ret->isAtomic = true;
@@ -1314,7 +1314,7 @@ class TranslateToFuzzReader {
       case i32: {
         switch (upTo(4)) {
           case 0: {
-            if (emitAtomics) {
+            if (features.hasAtomics()) {
               return makeUnary({ pick(EqZInt32, ClzInt32, CtzInt32, PopcntInt32, ExtendS8Int32, ExtendS16Int32), make(i32) });
             } else {
               return makeUnary({ pick(EqZInt32, ClzInt32, CtzInt32, PopcntInt32), make(i32) });
@@ -1330,7 +1330,7 @@ class TranslateToFuzzReader {
       case i64: {
         switch (upTo(4)) {
           case 0: {
-            if (emitAtomics) {
+            if (features.hasAtomics()) {
               return makeUnary({ pick(ClzInt64, CtzInt64, PopcntInt64, ExtendS8Int64, ExtendS16Int64, ExtendS32Int64), make(i64) });
             } else {
               return makeUnary({ pick(ClzInt64, CtzInt64, PopcntInt64), make(i64) });
@@ -1465,7 +1465,7 @@ class TranslateToFuzzReader {
   }
 
   Expression* makeAtomic(Type type) {
-    if (!emitAtomics || (type != i32 && type != i64)) return makeTrivial(type);
+    if (!features.hasAtomics() || (type != i32 && type != i64)) return makeTrivial(type);
     wasm.memory.shared = true;
     if (type == i32 && oneIn(2)) {
       if (ATOMIC_WAITS && oneIn(2)) {

src/tools/optimization-options.h

@@ -14,22 +14,20 @@
  * limitations under the License.
  */
 
-#include "support/command-line.h"
+#include "feature-options.h"
 
 //
 // Shared optimization options for commandline tools
 //
 
 namespace wasm {
 
-struct OptimizationOptions : public Options {
+struct OptimizationOptions : public FeatureOptions {
   static constexpr const char* DEFAULT_OPT_PASSES = "O";
 
   std::vector<std::string> passes;
-  PassOptions passOptions;
-  FeatureSet features = Feature::Atomics;
 
-  OptimizationOptions(const std::string& command, const std::string& description) : Options(command, description) {
+  OptimizationOptions(const std::string& command, const std::string& description) : FeatureOptions(command, description) {
     (*this).add("", "-O", "execute default optimization passes",
                 Options::Arguments::Zero,
                 [this](Options*, const std::string&) {
@@ -132,7 +130,7 @@ struct OptimizationOptions : public Options {
   void runPasses(Module& wasm) {
     PassRunner passRunner(&wasm, passOptions);
     if (debug) passRunner.setDebug(true);
-    passRunner.setFeatures(features);
+    passRunner.setFeatures(passOptions.features);
     for (auto& pass : passes) {
       if (pass == DEFAULT_OPT_PASSES) {
         passRunner.addDefaultOptimizationPasses();

src/tools/wasm-as.cpp

@@ -93,7 +93,7 @@ int main(int argc, const char *argv[]) {
 
   if (options.extra["validate"] != "none") {
     if (options.debug) std::cerr << "Validating..." << std::endl;
-    if (!wasm::WasmValidator().validate(wasm, Feature::All,
+    if (!wasm::WasmValidator().validate(wasm, FeatureSet::All,
          WasmValidator::Globally | (options.extra["validate"] == "web" ? WasmValidator::Web : 0))) {
       WasmPrinter::printModule(&wasm);
       Fatal() << "Error: input module is not valid.\n";

src/tools/wasm-opt.cpp

@@ -69,7 +69,6 @@ int main(int argc, const char* argv[]) {
   bool fuzzBinary = false;
   std::string extraFuzzCommand;
   bool translateToFuzz = false;
-  bool fuzzAtomics = true;
   bool fuzzPasses = false;
   std::string emitJSWrapper;
   std::string emitSpecWrapper;
@@ -106,9 +105,6 @@ int main(int argc, const char* argv[]) {
       .add("--translate-to-fuzz", "-ttf", "Translate the input into a valid wasm module *somehow*, useful for fuzzing",
            Options::Arguments::Zero,
            [&](Options *o, const std::string& arguments) { translateToFuzz = true; })
-      .add("--no-fuzz-atomics", "-nfa", "Disable generation of atomic opcodes with translate-to-fuzz (on by default)",
-           Options::Arguments::Zero,
-           [&](Options *o, const std::string& arguments) { fuzzAtomics = false; })
       .add("--fuzz-passes", "-fp", "Pick a random set of passes to run, useful for fuzzing. this depends on translate-to-fuzz (it picks the passes from the input)",
            Options::Arguments::Zero,
            [&](Options *o, const std::string& arguments) { fuzzPasses = true; })
@@ -134,11 +130,6 @@ int main(int argc, const char* argv[]) {
   options.parse(argc, argv);
 
   Module wasm;
-  // It should be safe to just always enable atomics in wasm-opt, because we
-  // don't expect any passes to accidentally generate atomic ops
-  FeatureSet features = Feature::Atomics;
-  // Same for MutableGlobals
-  features |= Feature::MutableGlobals;
 
   if (options.debug) std::cerr << "reading...\n";
 
@@ -160,7 +151,7 @@ int main(int argc, const char* argv[]) {
     }
 
     if (options.passOptions.validate) {
-      if (!WasmValidator().validate(wasm, features)) {
+      if (!WasmValidator().validate(wasm, options.getFeatures())) {
         WasmPrinter::printModule(&wasm);
         Fatal() << "error in validating input";
       }
@@ -171,9 +162,9 @@ int main(int argc, const char* argv[]) {
     if (fuzzPasses) {
       reader.pickPasses(options);
     }
-    reader.build(fuzzAtomics);
+    reader.build(options.getFeatures());
     if (options.passOptions.validate) {
-      if (!WasmValidator().validate(wasm, features)) {
+      if (!WasmValidator().validate(wasm, options.getFeatures())) {
         WasmPrinter::printModule(&wasm);
         std::cerr << "translate-to-fuzz must always generate a valid module";
         abort();
@@ -226,7 +217,7 @@ int main(int argc, const char* argv[]) {
     WasmBinaryBuilder parser(other, input, false);
     parser.read();
     if (options.passOptions.validate) {
-      bool valid = WasmValidator().validate(other, features);
+      bool valid = WasmValidator().validate(other, options.getFeatures());
       if (!valid) {
         WasmPrinter::printModule(&other);
       }
@@ -240,7 +231,7 @@ int main(int argc, const char* argv[]) {
     auto runPasses = [&]() {
       options.runPasses(*curr);
       if (options.passOptions.validate) {
-        bool valid = WasmValidator().validate(*curr, features);
+        bool valid = WasmValidator().validate(*curr, options.getFeatures());
         if (!valid) {
           WasmPrinter::printModule(&*curr);
         }