Inlining: exposed inlining thresholds as command-line parameters. (#2125)

* Inlining: exposed inlining thresholds as command-line parameters.

This will allow easier experimentation with optimal settings.

Also tweaked the default logic slightly to always inline single
caller functions up to a certain size.

The command-line arguments were tested to have the desired effect
for example by the Makefile change in this commit:
aardappel/lobster@39ae393
which in turn relies on:
emscripten-core/emscripten#8635

* Grouped inlining options & reverted defaults.

Now uses same defaults for inlining as before for the sake of
not having to redo a lot of tests.

Added FIXME to indicate that the current inlining logic needs
fixing.

* Fixed default values now pulled from code.

* clang-format

Author: aardappel

src/pass.h

@@ -56,6 +56,29 @@ struct PassRegistry {
   std::map<std::string, PassInfo> passInfos;
 };
 
+struct InliningOptions {
+  // Function size at which we always inline.
+  // Typically a size so small that after optimizations, the inlined code will
+  // be smaller than the call instruction itself. 2 is a safe number because
+  // there is no risk of things like
+  //  (func $reverse (param $x i32) (param $y i32)
+  //   (call $something (local.get $y) (local.get $x))
+  //  )
+  // in which case the reversing of the params means we'll possibly need
+  // a block and a temp local. But that takes at least 3 nodes, and 2 < 3.
+  // More generally, with 2 items we may have a local.get, but no way to
+  // require it to be saved instead of directly consumed.
+  Index alwaysInlineMaxSize = 2;
+  // Function size which we inline when functions are lightweight (no loops
+  // and calls) and we are doing aggressive optimisation for speed (-O3).
+  // In particular it's nice that with this limit we can inline the clamp
+  // functions (i32s-div, f64-to-int, etc.), that can affect perf.
+  Index flexibleInlineMaxSize = 20;
+  // Function size which we inline when there is only one caller.
+  // FIXME: this should logically be higher than flexibleInlineMaxSize.
+  Index oneCallerInlineMaxSize = 15;
+};
+
 struct PassOptions {
   // Run passes in debug mode, doing extra validation and timing checks.
   bool debug = false;
@@ -67,6 +90,8 @@ struct PassOptions {
   int optimizeLevel = 0;
   // 0, 1, 2 correspond to -O0, -Os, -Oz
   int shrinkLevel = 0;
+  // Tweak thresholds for the Inlining pass.
+  InliningOptions inlining;
   // Optimize assuming things like div by 0, bad load/store, will not trap.
   bool ignoreImplicitTraps = false;
   // Optimize assuming that the low 1K of memory is not valid memory for the

src/passes/Inlining.cpp

@@ -43,26 +43,6 @@
 
 namespace wasm {
 
-// A limit on how big a function to inline when being careful about size
-static const int CAREFUL_SIZE_LIMIT = 15;
-
-// A limit on how big a function to inline when being more flexible. In
-// particular it's nice that with this limit we can inline the clamp
-// functions (i32s-div, f64-to-int, etc.), that can affect perf.
-static const int FLEXIBLE_SIZE_LIMIT = 20;
-
-// A size so small that after optimizations, the inlined code will be
-// smaller than the call instruction itself. 2 is a safe number because
-// there is no risk of things like
-//  (func $reverse (param $x i32) (param $y i32)
-//   (call $something (local.get $y) (local.get $x))
-//  )
-// in which case the reversing of the params means we'll possibly need
-// a block and a temp local. But that takes at least 3 nodes, and 2 < 3.
-// More generally, with 2 items we may have a local.get, but no way to
-// require it to be saved instead of directly consumed.
-static const int INLINING_OPTIMIZING_WILL_DECREASE_SIZE_LIMIT = 2;
-
 // Useful into on a function, helping us decide if we can inline it
 struct FunctionInfo {
   std::atomic<Index> calls;
@@ -77,20 +57,25 @@ struct FunctionInfo {
     usedGlobally = false;
   }
 
+  // See pass.h for how defaults for these options were chosen.
   bool worthInlining(PassOptions& options) {
     // if it's big, it's just not worth doing (TODO: investigate more)
-    if (size > FLEXIBLE_SIZE_LIMIT) {
+    if (size > options.inlining.flexibleInlineMaxSize) {
       return false;
     }
     // if it's so small we have a guarantee that after we optimize the
     // size will not increase, inline it
-    if (size <= INLINING_OPTIMIZING_WILL_DECREASE_SIZE_LIMIT) {
+    if (size <= options.inlining.alwaysInlineMaxSize) {
       return true;
     }
     // if it has one use, then inlining it would likely reduce code size
     // since we are just moving code around, + optimizing, so worth it
     // if small enough that we are pretty sure its ok
-    if (calls == 1 && !usedGlobally && size <= CAREFUL_SIZE_LIMIT) {
+    // FIXME: move this check to be first in this function, since we should
+    // return true if oneCallerInlineMaxSize is bigger than
+    // flexibleInlineMaxSize (which it typically should be).
+    if (calls == 1 && !usedGlobally &&
+        size <= options.inlining.oneCallerInlineMaxSize) {
       return true;
     }
     // more than one use, so we can't eliminate it after inlining,

src/tools/optimization-options.h

@@ -123,6 +123,41 @@ struct OptimizationOptions : public ToolOptions {
            [this](Options* o, const std::string& argument) {
              passOptions.shrinkLevel = atoi(argument.c_str());
            })
+      .add("--always-inline-max-function-size",
+           "-aimfs",
+           "Max size of functions that are always inlined (default " +
+             std::to_string(InliningOptions().alwaysInlineMaxSize) +
+             ", which "
+             "is safe for use with -Os builds)",
+           Options::Arguments::One,
+           [this](Options* o, const std::string& argument) {
+             passOptions.inlining.alwaysInlineMaxSize =
+               static_cast<Index>(atoi(argument.c_str()));
+           })
+      .add("--flexible-inline-max-function-size",
+           "-fimfs",
+           "Max size of functions that are inlined when lightweight (no loops "
+           "or function calls) when optimizing aggressively for speed (-O3). "
+           "Default: " +
+             std::to_string(InliningOptions().flexibleInlineMaxSize),
+           Options::Arguments::One,
+           [this](Options* o, const std::string& argument) {
+             passOptions.inlining.flexibleInlineMaxSize =
+               static_cast<Index>(atoi(argument.c_str()));
+           })
+      .add("--one-caller-inline-max-function-size",
+           "-ocimfs",
+           "Max size of functions that are inlined when there is only one "
+           "caller (default " +
+             std::to_string(InliningOptions().oneCallerInlineMaxSize) +
+             "). Reason this is not unbounded is that some "
+             "implementations may have a hard time optimizing really large "
+             "functions",
+           Options::Arguments::One,
+           [this](Options* o, const std::string& argument) {
+             passOptions.inlining.oneCallerInlineMaxSize =
+               static_cast<Index>(atoi(argument.c_str()));
+           })
       .add("--ignore-implicit-traps",
            "-iit",
            "Optimize under the helpful assumption that no surprising traps "