Refactor optimization defaults (#1366)

kripken · web-flow · commit 692069c6eef6 · 2018-01-17T20:26:04.000-08:00
Followup to #1357. This moves the optimization settings into pass.h, and uses it from there in the various places. This also splits up huge lines from the tracing code, which put all block children (whose number can be arbitrarily large) on one line. This seems to have caused random errors on the bots, I suspect from overflowing a buffer. Anyhow, it's much more clear to split the lines at a reasonable length.
diff --git a/bin/binaryen.js b/bin/binaryen.js
diff --git a/bin/wasm.js b/bin/wasm.js
diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp
@@ -33,7 +33,6 @@
 #include "cfg/Relooper.h"
 #include "ir/utils.h"
 #include "shell-interface.h"
-#include "support/defaults.h"
 
 using namespace wasm;
 
@@ -72,9 +71,7 @@ static std::mutex BinaryenFunctionMutex;
 static std::mutex BinaryenFunctionTypeMutex;
 
 // Optimization options
-static int optimizeLevel = BINARYEN_DEFAULT_OPTIMIZE_LEVEL;
-static int shrinkLevel = BINARYEN_DEFAULT_SHRINK_LEVEL;
-static bool debugInfo = BINARYEN_DEFAULT_DEBUG_INFO;
+static PassOptions globalPassOptions = PassOptions::getWithDefaultOptimizationOptions();
 
 // Tracing support
 
@@ -385,6 +382,7 @@ BinaryenExpressionRef BinaryenBlock(BinaryenModuleRef module, const char* name,
     std::cout << "    BinaryenExpressionRef children[] = { ";
     for (BinaryenIndex i = 0; i < numChildren; i++) {
       if (i > 0) std::cout << ", ";
+      if (i % 6 == 5) std::cout << "\n       "; // don't create hugely long lines
       std::cout << "expressions[" << expressions[children[i]] << "]";
     }
     if (numChildren == 0) std::cout << "0"; // ensure the array is not empty, otherwise a compiler error on VS
@@ -2013,9 +2011,7 @@ void BinaryenModuleOptimize(BinaryenModuleRef module) {
 
   Module* wasm = (Module*)module;
   PassRunner passRunner(wasm);
-  passRunner.options.optimizeLevel = optimizeLevel;
-  passRunner.options.shrinkLevel = shrinkLevel;
-  passRunner.options.debugInfo = debugInfo;
+  passRunner.options = globalPassOptions;
   passRunner.addDefaultOptimizationPasses();
   passRunner.run();
 }
@@ -2025,47 +2021,47 @@ int BinaryenGetOptimizeLevel() {
     std::cout << "  BinaryenGetOptimizeLevel();\n";
   }
 
-  return optimizeLevel;
+  return globalPassOptions.optimizeLevel;
 }
 
 void BinaryenSetOptimizeLevel(int level) {
   if (tracing) {
     std::cout << "  BinaryenSetOptimizeLevel(" << level << ");\n";
   }
 
-  optimizeLevel = level;
+  globalPassOptions.optimizeLevel = level;
 }
 
 int BinaryenGetShrinkLevel() {
   if (tracing) {
     std::cout << "  BinaryenGetShrinkLevel();\n";
   }
 
-  return shrinkLevel;
+  return globalPassOptions.shrinkLevel;
 }
 
 void BinaryenSetShrinkLevel(int level) {
   if (tracing) {
     std::cout << "  BinaryenSetShrinkLevel(" << level << ");\n";
   }
 
-  shrinkLevel = level;
+  globalPassOptions.shrinkLevel = level;
 }
 
 int BinaryenGetDebugInfo() {
   if (tracing) {
     std::cout << "  BinaryenGetDebugInfo();\n";
   }
 
-  return debugInfo;
+  return globalPassOptions.debugInfo;
 }
 
 void BinaryenSetDebugInfo(int on) {
   if (tracing) {
     std::cout << "  BinaryenSetDebugInfo(" << on << ");\n";
   }
 
-  debugInfo = bool(on);
+  globalPassOptions.debugInfo = bool(on);
 }
 
 void BinaryenModuleRunPasses(BinaryenModuleRef module, const char **passes, BinaryenIndex numPasses) {
@@ -2083,9 +2079,7 @@ void BinaryenModuleRunPasses(BinaryenModuleRef module, const char **passes, Bina
 
   Module* wasm = (Module*)module;
   PassRunner passRunner(wasm);
-  passRunner.options.optimizeLevel = optimizeLevel;
-  passRunner.options.shrinkLevel = shrinkLevel;
-  passRunner.options.debugInfo = debugInfo;
+  passRunner.options = globalPassOptions;
   for (BinaryenIndex i = 0; i < numPasses; i++) {
     passRunner.add(passes[i]);
   }
@@ -2099,7 +2093,7 @@ void BinaryenModuleAutoDrop(BinaryenModuleRef module) {
 
   Module* wasm = (Module*)module;
   PassRunner passRunner(wasm);
-  passRunner.options.debugInfo = debugInfo;
+  passRunner.options = globalPassOptions;
   passRunner.add<AutoDrop>();
   passRunner.run();
 }
@@ -2112,7 +2106,7 @@ size_t BinaryenModuleWrite(BinaryenModuleRef module, char* output, size_t output
   Module* wasm = (Module*)module;
   BufferWithRandomAccess buffer(false);
   WasmBinaryWriter writer(wasm, buffer, false);
-  writer.setNamesSection(debugInfo);
+  writer.setNamesSection(globalPassOptions.debugInfo);
   writer.write();
   size_t bytes = std::min(buffer.size(), outputSize);
   std::copy_n(buffer.begin(), bytes, output);
@@ -2254,9 +2248,7 @@ void BinaryenFunctionOptimize(BinaryenFunctionRef func, BinaryenModuleRef module
 
   Module* wasm = (Module*)module;
   PassRunner passRunner(wasm);
-  passRunner.options.optimizeLevel = optimizeLevel;
-  passRunner.options.shrinkLevel = shrinkLevel;
-  passRunner.options.debugInfo = debugInfo;
+  passRunner.options = globalPassOptions;
   passRunner.addDefaultOptimizationPasses();
   passRunner.runOnFunction((Function*)func);
 }
@@ -2275,9 +2267,7 @@ void BinaryenFunctionRunPasses(BinaryenFunctionRef func, BinaryenModuleRef modul
 
   Module* wasm = (Module*)module;
   PassRunner passRunner(wasm);
-  passRunner.options.optimizeLevel = optimizeLevel;
-  passRunner.options.shrinkLevel = shrinkLevel;
-  passRunner.options.debugInfo = debugInfo;
+  passRunner.options = globalPassOptions;
   for (BinaryenIndex i = 0; i < numPasses; i++) {
     passRunner.add(passes[i]);
   }
diff --git a/src/binaryen-c.h b/src/binaryen-c.h
@@ -48,7 +48,6 @@
 #include <stdint.h>
 
 #include "compiler-support.h"
-#include "support/defaults.h"
 
 #ifdef __cplusplus
 extern "C" {
diff --git a/src/pass.h b/src/pass.h
@@ -63,6 +63,18 @@ struct PassOptions {
   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
+
+  void setDefaultOptimizationOptions() {
+    // -Os is our default
+    optimizeLevel = 2;
+    shrinkLevel = 1;
+  }
+
+  static PassOptions getWithDefaultOptimizationOptions() {
+    PassOptions ret;
+    ret.setDefaultOptimizationOptions();
+    return ret;
+  }
 };
 
 //
diff --git a/src/support/defaults.h b/src/support/defaults.h
diff --git a/src/tools/optimization-options.h b/src/tools/optimization-options.h
@@ -15,7 +15,6 @@
  */
 
 #include "support/command-line.h"
-#include "support/defaults.h"
 
 //
 // Shared optimization options for commandline tools
@@ -34,8 +33,7 @@ struct OptimizationOptions : public Options {
     (*this).add("", "-O", "execute default optimization passes",
                 Options::Arguments::Zero,
                 [this](Options*, const std::string&) {
-                  passOptions.optimizeLevel = BINARYEN_DEFAULT_OPTIMIZE_LEVEL;
-                  passOptions.shrinkLevel = BINARYEN_DEFAULT_SHRINK_LEVEL;
+                  passOptions.setDefaultOptimizationOptions();
                   passes.push_back(DEFAULT_OPT_PASSES);
                 })
            .add("", "-O0", "execute no optimization passes",
diff --git a/test/binaryen.js/kitchen-sink.js.txt b/test/binaryen.js/kitchen-sink.js.txt
@@ -1418,7 +1418,22 @@ getExpressionInfo(f32.const)={"id":15,"type":3,"value":8.5}
   BinaryenConstGetValueF64(expressions[250]);
 getExpressionInfo(f64.const)={"id":15,"type":4,"value":9.5}
   {
-], expressions[160], expressions[163], expressions[166], expressions[169], expressions[172], expressions[175], expressions[178], expressions[181], expressions[184], expressions[187], expressions[190], expressions[191], expressions[192], expressions[193], expressions[195], expressions[197], expressions[198], expressions[200], expressions[202], expressions[203], expressions[204], expressions[206], expressions[212], expressions[217], expressions[224], expressions[226], expressions[228], expressions[231], expressions[233], expressions[235], expressions[237], expressions[239], expressions[240], expressions[241], expressions[242], expressions[244], expressions[245], expressions[246] };
+    BinaryenExpressionRef children[] = { expressions[24], expressions[26], expressions[28], expressions[30], expressions[32], 
+       expressions[34], expressions[36], expressions[38], expressions[40], expressions[42], expressions[44], 
+       expressions[46], expressions[48], expressions[50], expressions[52], expressions[54], expressions[56], 
+       expressions[58], expressions[60], expressions[62], expressions[64], expressions[66], expressions[68], 
+       expressions[70], expressions[72], expressions[74], expressions[76], expressions[78], expressions[80], 
+       expressions[82], expressions[84], expressions[86], expressions[88], expressions[90], expressions[92], 
+       expressions[94], expressions[97], expressions[100], expressions[103], expressions[106], expressions[109], 
+       expressions[112], expressions[115], expressions[118], expressions[121], expressions[124], expressions[127], 
+       expressions[130], expressions[133], expressions[136], expressions[139], expressions[142], expressions[145], 
+       expressions[148], expressions[151], expressions[154], expressions[157], expressions[160], expressions[163], 
+       expressions[166], expressions[169], expressions[172], expressions[175], expressions[178], expressions[181], 
+       expressions[184], expressions[187], expressions[190], expressions[191], expressions[192], expressions[193], 
+       expressions[195], expressions[197], expressions[198], expressions[200], expressions[202], expressions[203], 
+       expressions[204], expressions[206], expressions[212], expressions[217], expressions[224], expressions[226], 
+       expressions[228], expressions[231], expressions[233], expressions[235], expressions[237], expressions[239], 
+       expressions[240], expressions[241], expressions[242], expressions[244], expressions[245], expressions[246] };
     expressions[251] = BinaryenBlock(the_module, "the-value", children, 95, 0);
   }
   expressions[252] = BinaryenDrop(the_module, expressions[251]);
diff --git a/test/example/c-api-kitchen-sink.txt b/test/example/c-api-kitchen-sink.txt
@@ -1368,7 +1368,22 @@ int main() {
  (f32.const -33.61199951171875)
 )
   {
-    BinaryenExpressionRef children[] = { expressions[34], expressions[36], expressions[38], expressions[40], expressions[42], expressions[44], expressions[46], expressions[48], expressions[50], expressions[52], expressions[54], expressions[56], expressions[58], expressions[60], expressions[62], expressions[64], expressions[66], expressions[68], expressions[70], expressions[72], expressions[74], expressions[76], expressions[78], expressions[80], expressions[82], expressions[84], expressions[86], expressions[88], expressions[90], expressions[92], expressions[94], expressions[96], expressions[98], expressions[100], expressions[102], expressions[104], expressions[107], expressions[110], expressions[113], expressions[116], expressions[119], expressions[122], expressions[125], expressions[128], expressions[131], expressions[134], expressions[137], expressions[140], expressions[143], expressions[146], expressions[149], expressions[152], expressions[155], expressions[158], expressions[161], expressions[164], expressions[167], expressions[170], expressions[173], expressions[176], expressions[179], expressions[182], expressions[185], expressions[188], expressions[191], expressions[194], expressions[197], expressions[200], expressions[201], expressions[202], expressions[203], expressions[205], expressions[207], expressions[208], expressions[210], expressions[212], expressions[213], expressions[214], expressions[216], expressions[218], expressions[221], expressions[224], expressions[226], expressions[228], expressions[231], expressions[233], expressions[235], expressions[237], expressions[239], expressions[240], expressions[241], expressions[242], expressions[244], expressions[245], expressions[246] };
+    BinaryenExpressionRef children[] = { expressions[34], expressions[36], expressions[38], expressions[40], expressions[42], 
+       expressions[44], expressions[46], expressions[48], expressions[50], expressions[52], expressions[54], 
+       expressions[56], expressions[58], expressions[60], expressions[62], expressions[64], expressions[66], 
+       expressions[68], expressions[70], expressions[72], expressions[74], expressions[76], expressions[78], 
+       expressions[80], expressions[82], expressions[84], expressions[86], expressions[88], expressions[90], 
+       expressions[92], expressions[94], expressions[96], expressions[98], expressions[100], expressions[102], 
+       expressions[104], expressions[107], expressions[110], expressions[113], expressions[116], expressions[119], 
+       expressions[122], expressions[125], expressions[128], expressions[131], expressions[134], expressions[137], 
+       expressions[140], expressions[143], expressions[146], expressions[149], expressions[152], expressions[155], 
+       expressions[158], expressions[161], expressions[164], expressions[167], expressions[170], expressions[173], 
+       expressions[176], expressions[179], expressions[182], expressions[185], expressions[188], expressions[191], 
+       expressions[194], expressions[197], expressions[200], expressions[201], expressions[202], expressions[203], 
+       expressions[205], expressions[207], expressions[208], expressions[210], expressions[212], expressions[213], 
+       expressions[214], expressions[216], expressions[218], expressions[221], expressions[224], expressions[226], 
+       expressions[228], expressions[231], expressions[233], expressions[235], expressions[237], expressions[239], 
+       expressions[240], expressions[241], expressions[242], expressions[244], expressions[245], expressions[246] };
     expressions[247] = BinaryenBlock(the_module, "the-value", children, 95, BinaryenTypeAuto());
   }
   expressions[248] = BinaryenDrop(the_module, expressions[247]);
