Inlining: Slight reordering of options (#3308)

This simplifies the three size-related inlining flags, so that their
meanings are clearer. Also improve the comments and put them
in a consistent order in both files.

This should not make any difference in general, except that it is now
possible to have oneCallerInlineMaxSize > flexibleInlineMaxSize,
and we will inline a function with one caller if
flexibleInlineMaxSize < FUNCTION_SIZE <= oneCallerInlineMaxSize
which we would not before. As the defaults of the flags didn't fit that
case, this should not change anything for anyone not passing in
those specific inlining flags. For people that pass in those flags, this
PR makes more things possible.

Resolves the FIXME in that code, and is a refactoring before some
more inlining work I have planned.

2 files changed, 14 insertions, 19 deletions

diff --git a/src/pass.h b/src/pass.h
index 3b055cee9..7b42e3719 100644
--- a/src/pass.h
+++ b/src/pass.h
@@ -69,14 +69,15 @@ struct InliningOptions {
   // 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;
+  // Function size above which we never inline, ignoring the various flexible
+  // factors (like whether we are optimizing for size or speed) that could
+  // influence us.
+  // This is checked after alwaysInlineMaxSize and oneCallerInlineMaxSize, but
+  // the order normally won't matter.
+  Index flexibleInlineMaxSize = 20;
   // Loops usually mean the function does heavy work, so the call overhead
   // is not significant and we do not inline such functions by default.
   bool allowFunctionsWithLoops = false;
diff --git a/src/passes/Inlining.cpp b/src/passes/Inlining.cpp
index 5c94755d1..bcab7318f 100644
--- a/src/passes/Inlining.cpp
+++ b/src/passes/Inlining.cpp
@@ -17,8 +17,6 @@
 //
 // Inlining.
 //
-// This uses some simple heuristics to decide when to inline.
-//
 // Two versions are provided: inlining and inlining-optimizing. You
 // probably want the optimizing version, which will optimize locations
 // we inlined into, as inlining by itself creates a block to house the
@@ -62,25 +60,21 @@ struct FunctionInfo {
 
   // 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 > 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 it's small enough that we always want to inline such things, do so.
     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
-    // 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 it has one use, then inlining it would likely reduce code size, at
+    // least for reasonable function sizes.
     if (refs == 1 && !usedGlobally &&
         size <= options.inlining.oneCallerInlineMaxSize) {
       return true;
     }
+    // If it's so big that we have no flexible options that could allow it,
+    // do not inline.
+    if (size > options.inlining.flexibleInlineMaxSize) {
+      return false;
+    }
     // More than one use, so we can't eliminate it after inlining,
     // so only worth it if we really care about speed and don't care
     // about size. First, check if it has calls. In that case it is not