Change default feature set to MVP (#1993)

In the absence of the target features section or command line flags. When there are command line flags, it is an error if they do not exactly match the target features section, except if --detect-features has been provided.

Also adds a --print-features pass to print the command line flags for all enabled options and uses it to make the feature tests more rigorous.

10 files changed, 63 insertions, 11 deletions

diff --git a/src/passes/CMakeLists.txt b/src/passes/CMakeLists.txt
index 10fdc7fe4..8a4b04de8 100644
--- a/src/passes/CMakeLists.txt
+++ b/src/passes/CMakeLists.txt
@@ -40,6 +40,7 @@ SET(passes_SOURCES
   Precompute.cpp
   Print.cpp
   PrintCallGraph.cpp
+  PrintFeatures.cpp
   StackIR.cpp
   Strip.cpp
   StripTargetFeatures.cpp
diff --git a/src/passes/PrintFeatures.cpp b/src/passes/PrintFeatures.cpp
new file mode 100644
index 000000000..9c7172eee
--- /dev/null
+++ b/src/passes/PrintFeatures.cpp
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2019 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.
+ */
+
+//
+// Print out the feature options corresponding to enabled features
+//
+
+#include "wasm.h"
+#include "wasm-features.h"
+#include "pass.h"
+
+namespace wasm {
+
+struct PrintFeatures : public Pass {
+  void run(PassRunner* runner, Module* module) override {
+    module->features.iterFeatures([](FeatureSet::Feature f) {
+      std::cout << "--enable-" << FeatureSet::toString(f) << std::endl;
+    });
+  }
+};
+
+Pass* createPrintFeaturesPass() {
+  return new PrintFeatures();
+}
+
+} // namespace wasm
diff --git a/src/passes/StripTargetFeatures.cpp b/src/passes/StripTargetFeatures.cpp
index 645fbb11c..8eb7b0b75 100644
--- a/src/passes/StripTargetFeatures.cpp
+++ b/src/passes/StripTargetFeatures.cpp
@@ -16,8 +16,6 @@
 
 #include "pass.h"
 
-using namespace std;
-
 namespace wasm {
 
 struct StripTargetFeatures : public Pass {
diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp
index fd5359206..41d2026bc 100644
--- a/src/passes/pass.cpp
+++ b/src/passes/pass.cpp
@@ -112,6 +112,7 @@ void PassRegistry::registerPasses() {
   registerPass("precompute-propagate", "computes compile-time evaluatable expressions and propagates them through locals", createPrecomputePropagatePass);
   registerPass("print", "print in s-expression format", createPrinterPass);
   registerPass("print-minified", "print in minified s-expression format", createMinifiedPrinterPass);
+  registerPass("print-features", "print options for enabled features", createPrintFeaturesPass);
   registerPass("print-full", "print in full s-expression format", createFullPrinterPass);
   registerPass("print-call-graph", "print call graph", createPrintCallGraphPass);
   registerPass("print-stack-ir", "print out Stack IR (useful for internal debugging)", createPrintStackIRPass);
diff --git a/src/passes/passes.h b/src/passes/passes.h
index e4111c43b..af9141ac9 100644
--- a/src/passes/passes.h
+++ b/src/passes/passes.h
@@ -69,6 +69,7 @@ Pass* createPrecomputePass();
 Pass* createPrecomputePropagatePass();
 Pass* createPrinterPass();
 Pass* createPrintCallGraphPass();
+Pass* createPrintFeaturesPass();
 Pass* createPrintStackIRPass();
 Pass* createRelooperJumpThreadingPass();
 Pass* createRemoveNonJSOpsPass();
diff --git a/src/tools/tool-options.h b/src/tools/tool-options.h
index dd585612f..02572d72e 100644
--- a/src/tools/tool-options.h
+++ b/src/tools/tool-options.h
@@ -92,12 +92,12 @@ struct ToolOptions : public Options {
   void applyFeatures(Module& module) {
     if (hasFeatureOptions) {
       if (!detectFeatures && module.hasFeaturesSection) {
-        FeatureSet optionsFeatures = FeatureSet::All;
+        FeatureSet optionsFeatures = FeatureSet::MVP;
         optionsFeatures.enable(enabledFeatures);
         optionsFeatures.disable(disabledFeatures);
-        if (!(module.features <= optionsFeatures)) {
-          Fatal() << "module uses features not explicitly specified, "
-                  << "use --detect-features to resolve";
+        if (module.features != optionsFeatures) {
+          Fatal() << "module features do not match specified features. "
+                  << "Use --detect-features to resolve.";
         }
       }
       module.features.enable(enabledFeatures);
@@ -108,7 +108,7 @@ struct ToolOptions : public Options {
 private:
   bool hasFeatureOptions = false;
   bool detectFeatures = false;
-  FeatureSet enabledFeatures = FeatureSet::All;
+  FeatureSet enabledFeatures = FeatureSet::MVP;
   FeatureSet disabledFeatures = FeatureSet::MVP;
 };
 
diff --git a/src/tools/wasm-reduce.cpp b/src/tools/wasm-reduce.cpp
index 1f58da0d5..e064abbcb 100644
--- a/src/tools/wasm-reduce.cpp
+++ b/src/tools/wasm-reduce.cpp
@@ -271,7 +271,9 @@ struct Reducer : public WalkerPass<PostWalker<Reducer, UnifiedExpressionVisitor<
       // try both combining with a generic shrink (so minor pass overhead is compensated for), and without
       for (auto pass : passes) {
         std::string currCommand = Path::getBinaryenBinaryTool("wasm-opt") + " ";
-        currCommand += working + " -o " + test + " " + pass;
+        // TODO(tlively): -all should be replaced with an option to use the
+        // existing feature set, once implemented.
+        currCommand += working + " -all -o " + test + " " + pass;
         if (debugInfo) currCommand += " -g ";
         if (verbose) std::cerr << "|    trying pass command: " << currCommand << "\n";
         if (!ProgramResult(currCommand).failed()) {
@@ -321,6 +323,7 @@ struct Reducer : public WalkerPass<PostWalker<Reducer, UnifiedExpressionVisitor<
     Module wasm;
     ModuleReader reader;
     reader.read(working, *module);
+    wasm.features = FeatureSet::All;
     builder = make_unique<Builder>(*module);
     setModule(module.get());
   }
@@ -999,7 +1002,9 @@ int main(int argc, const char* argv[]) {
   std::cerr << "|checking that command has expected behavior on canonicalized (read-written) binary\n";
   {
     // read and write it
-    auto cmd = Path::getBinaryenBinaryTool("wasm-opt") + " " + input + " -o " + test;
+    // TODO(tlively): -all should be replaced with an option to use the existing
+    // feature set, once implemented.
+    auto cmd = Path::getBinaryenBinaryTool("wasm-opt") + " " + input + " -all -o " + test;
     if (!binary) cmd += " -S";
     ProgramResult readWrite(cmd);
     if (readWrite.failed()) {
diff --git a/src/wasm-features.h b/src/wasm-features.h
index a1e7a321d..6ef704f82 100644
--- a/src/wasm-features.h
+++ b/src/wasm-features.h
@@ -88,6 +88,14 @@ struct FeatureSet {
     return !(features & ~other.features);
   }
 
+  bool operator==(const FeatureSet& other) const {
+    return *this <= other && other <= *this;
+  }
+
+  bool operator!=(const FeatureSet& other) const {
+    return !(*this == other);
+  }
+
   FeatureSet& operator|=(const FeatureSet& other) {
     features |= other.features;
     return *this;
diff --git a/src/wasm.h b/src/wasm.h
index 2171eeb12..f4180f7a0 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -928,7 +928,7 @@ public:
   // respected regardless of the value of`hasFeaturesSection`.
   // `hasFeaturesSection` means we read a features section and will emit one
   // too.
-  FeatureSet features = FeatureSet::All;
+  FeatureSet features = FeatureSet::MVP;
   bool hasFeaturesSection = false;
 
   MixedArena allocator;
diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp
index b969a1fbd..837dcbea1 100644
--- a/src/wasm/wasm-emscripten.cpp
+++ b/src/wasm/wasm-emscripten.cpp
@@ -973,7 +973,6 @@ std::string EmscriptenGlueGenerator::generateEmscriptenMetadata(
 
   meta << "  \"features\": [";
   commaFirst = true;
-  meta << nextElement() << "\"--mvp-features\"";
   wasm.features.iterFeatures([&](FeatureSet::Feature f) {
     meta << nextElement() << "\"--enable-" << FeatureSet::toString(f) << '"';
   });