Separate inlining-heuristic code into its own file. Implement a simple statement-counting inlining heuristic.

gen/function-inlining.cpp

@@ -0,0 +1,196 @@
+//===-- function-inlining.cpp ---------------------------------------------===//
+//
+//                         LDC – the LLVM D compiler
+//
+// This file is distributed under the BSD-style LDC license. See the LICENSE
+// file for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gen/function-inlining.h"
+
+#include "declaration.h"
+#include "globals.h"
+#include "id.h"
+#include "module.h"
+#include "statement.h"
+#include "template.h"
+#include "gen/logger.h"
+#include "gen/optimizer.h"
+#include "gen/recursivevisitor.h"
+#include "gen/uda.h"
+
+namespace {
+
+/// An ASTVisitor that checks whether the number of statements is larger than a
+/// certain number.
+struct MoreThanXStatements : public StoppableVisitor {
+  /// Are there more or fewer statements than `threshold`?.
+  unsigned threshold;
+  /// The statement count.
+  unsigned count;
+
+  MoreThanXStatements(unsigned X) : threshold(X), count(0) {}
+
+  using StoppableVisitor::visit;
+
+  void visit(Statement *stmt) override {
+    count++;
+    if (count > threshold)
+      stop = true;
+  }
+  void visit(Expression *exp) override {}
+  void visit(Declaration *decl) override {}
+  void visit(Initializer *init) override {}
+  void visit(Dsymbol *) override {}
+};
+
+// Use a heuristic to determine if it could make sense to inline this fdecl.
+// Note: isInlineCandidate is called _before_ semantic3 analysis of fdecl.
+bool isInlineCandidate(FuncDeclaration &fdecl) {
+  // Giving maximum inlining potential to LLVM should be possible, but we
+  // restrict it to save some compile time.
+  // return true;
+
+  // TODO: make the heuristic more sophisticated?
+  // In the end, LLVM will make the decision whether to _actually_ inline.
+  // The statement count threshold is completely arbitrary. Also, all
+  // statements are weighed the same.
+
+  unsigned statementThreshold = 10;
+  MoreThanXStatements statementCounter(statementThreshold);
+  RecursiveWalker walker(&statementCounter, false);
+  fdecl.fbody->accept(&walker);
+
+  IF_LOG Logger::println("Contains %u statements or more (threshold = %u).",
+                         statementCounter.count, statementThreshold);
+  return statementCounter.count <= statementThreshold;
+}
+
+} // end anonymous namespace
+
+bool alreadyOrWillBeDefined(FuncDeclaration &fdecl) {
+  for (FuncDeclaration *f = &fdecl; f;) {
+    if (!f->isInstantiated() && f->inNonRoot()) {
+      return false;
+    }
+    if (f->isNested()) {
+      f = f->toParent2()->isFuncDeclaration();
+    } else {
+      break;
+    }
+  }
+  return true;
+}
+
+bool defineAsExternallyAvailable(FuncDeclaration &fdecl) {
+  IF_LOG Logger::println("Enter defineAsExternallyAvailable");
+  LOG_SCOPE
+
+#if LDC_LLVM_VER < 307
+  // Pre-3.7, cross-module inlining is disabled completely.
+  // See the commandline flag definition for more details.
+  IF_LOG Logger::println("LLVM < 3.7: Cross-module inlining disabled.");
+  return false;
+#endif
+
+  // Implementation note: try to do cheap checks first.
+
+  if (fdecl.neverInline || fdecl.inlining == PINLINEnever) {
+    IF_LOG Logger::println("pragma(inline, false) specified");
+    return false;
+  }
+
+  // pragma(inline, true) functions will be inlined even at -O0
+  if (fdecl.inlining == PINLINEalways) {
+    IF_LOG Logger::println(
+        "pragma(inline, true) specified, overrides cmdline flags");
+  } else if (!willCrossModuleInline()) {
+    IF_LOG Logger::println("Commandline flags indicate no inlining");
+    return false;
+  }
+
+  if (fdecl.isUnitTestDeclaration()) {
+    IF_LOG Logger::println("isUnitTestDeclaration() == true");
+    return false;
+  }
+  if (fdecl.isFuncAliasDeclaration()) {
+    IF_LOG Logger::println("isFuncAliasDeclaration() == true");
+    return false;
+  }
+  if (!fdecl.fbody) {
+    IF_LOG Logger::println("No function body available for inlining");
+    return false;
+  }
+
+  // TODO: Fix inlining functions from object.d. Currently errors because of
+  // TypeInfo type-mismatch issue (TypeInfo classes get special treatment by the
+  // compiler). To start working on it: comment-out this check and druntime will
+  // fail to compile.
+  if (fdecl.getModule()->ident == Id::object) {
+    IF_LOG Logger::println("Inlining of object.d functions is disabled");
+    return false;
+  }
+
+  if (fdecl.semanticRun >= PASSsemantic3) {
+    // If semantic analysis has come this far, the function will be defined
+    // elsewhere and should not get the available_externally attribute from
+    // here.
+    // TODO: This check prevents inlining of nested functions.
+    IF_LOG Logger::println("Semantic analysis already completed");
+    return false;
+  }
+
+  if (alreadyOrWillBeDefined(fdecl)) {
+    // This check is needed because of ICEs happening because of unclear issues
+    // upon changing the codegen order without this check.
+    IF_LOG Logger::println("Function will be defined later.");
+    return false;
+  }
+
+  // Weak-linkage functions can not be inlined.
+  if (hasWeakUDA(&fdecl)) {
+    IF_LOG Logger::println("@weak functions cannot be inlined.");
+    return false;
+  }
+
+  if (fdecl.inlining != PINLINEalways && !isInlineCandidate(fdecl))
+    return false;
+
+  IF_LOG Logger::println("Potential inlining candidate");
+
+  {
+    IF_LOG Logger::println("Do semantic analysis");
+    LOG_SCOPE
+
+    // The inlining is aggressive and may give semantic errors that are
+    // forward referencing errors. Simply avoid those cases for inlining.
+    unsigned errors = global.startGagging();
+    global.gaggedForInlining = true;
+
+    bool semantic_error = false;
+    if (fdecl.functionSemantic3()) {
+      Module::runDeferredSemantic3();
+    } else {
+      IF_LOG Logger::println("Failed functionSemantic3.");
+      semantic_error = true;
+    }
+
+    global.gaggedForInlining = false;
+    if (global.endGagging(errors) || semantic_error) {
+      IF_LOG Logger::println("Errors occured during semantic analysis.");
+      return false;
+    }
+    assert(fdecl.semanticRun >= PASSsemantic3done);
+  }
+
+  // FuncDeclaration::naked is set by the AsmParser during semantic3 analysis,
+  // and so this check can only be done at this late point.
+  if (fdecl.naked) {
+    IF_LOG Logger::println("Naked asm functions cannot be inlined.");
+    return false;
+  }
+
+  IF_LOG Logger::println("defineAsExternallyAvailable? Yes.");
+  return true;
+}

gen/function-inlining.h

@@ -0,0 +1,29 @@
+//===-- gen/function-inlining.h ---------------------------------*- C++ -*-===//
+//
+//                         LDC – the LLVM D compiler
+//
+// This file is distributed under the BSD-style LDC license. See the LICENSE
+// file for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Determines whether a function is fit for inlining or not.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LDC_GEN_FUNCTION_INLINING_H
+#define LDC_GEN_FUNCTION_INLINING_H
+
+class FuncDeclaration;
+
+/// Check whether the frontend knows that the function is already defined
+/// in some other module (see DMD's FuncDeclaration::toObjFile).
+bool alreadyOrWillBeDefined(FuncDeclaration &fdecl);
+
+/// Returns whether `fdecl` should be emitted with externally_available
+/// linkage to make it available for inlining.
+///
+/// If true, `semantic3` will have been run on the declaration.
+bool defineAsExternallyAvailable(FuncDeclaration &fdecl);
+
+#endif

gen/functions.cpp

@@ -22,6 +22,7 @@
 #include "gen/arrays.h"
 #include "gen/classes.h"
 #include "gen/dvalue.h"
+#include "gen/function-inlining.h"
 #include "gen/inlineir.h"
 #include "gen/irstate.h"
 #include "gen/linkage.h"
@@ -42,154 +43,6 @@
 #include "llvm/IR/CFG.h"
 #include <iostream>
 
-////////////////////////////////////////////////////////////////////////////////
-// Function inlining
-// TODO: move to its own file?
-namespace {
-
-// Use a heuristic to determine if it could make sense to inline this fdecl.
-// Note: isInlineCandidate is called _before_ semantic3 analysis of fdecl.
-bool isInlineCandidate(FuncDeclaration &fdecl) {
-  if (fdecl.inlining == PINLINEalways)
-    return true;
-
-  return true;
-}
-
-/// Check whether the frontend knows that the function is already defined
-/// in some other module (see DMD's FuncDeclaration::toObjFile).
-bool alreadyOrWillBeDefined(FuncDeclaration &fdecl) {
-  for (FuncDeclaration *f = &fdecl; f;) {
-    if (!f->isInstantiated() && f->inNonRoot()) {
-      return false;
-    }
-    if (f->isNested()) {
-      f = f->toParent2()->isFuncDeclaration();
-    } else {
-      break;
-    }
-  }
-  return true;
-}
-
-
-/// Returns whether `fdecl` should be emitted with externally_available
-/// linkage to make it available for inlining.
-///
-/// If true, `semantic3` will have been run on the declaration.
-bool defineAsExternallyAvailable(FuncDeclaration &fdecl) {
-  IF_LOG Logger::println("Enter defineAsExternallyAvailable");
-  LOG_SCOPE
-
-#if LDC_LLVM_VER < 307
-  // Pre-3.7, cross-module inlining is disabled completely.
-  // See the commandline flag definition for more details.
-  IF_LOG Logger::println("LLVM < 3.7: Cross-module inlining disabled.");
-  return false;
-#endif
-
-  // Implementation note: try to do cheap checks first.
-
-  if (fdecl.neverInline || fdecl.inlining == PINLINEnever) {
-    IF_LOG Logger::println("pragma(inline, false) specified");
-    return false;
-  }
-
-  // pragma(inline, true) functions will be inlined even at -O0
-  if (fdecl.inlining == PINLINEalways) {
-    IF_LOG Logger::println("pragma(inline, true) specified, overrides cmdline flags");
-  }
-  else if (!willCrossModuleInline()) {
-    IF_LOG Logger::println("Commandline flags indicate no inlining");
-    return false;
-  }
-
-  if (fdecl.isUnitTestDeclaration()) {
-    IF_LOG Logger::println("isUnitTestDeclaration() == true");
-    return false;
-  }
-  if (fdecl.isFuncAliasDeclaration()) {
-    IF_LOG Logger::println("isFuncAliasDeclaration() == true");
-    return false;
-  }
-  if (!fdecl.fbody) {
-    IF_LOG Logger::println("No function body available for inlining");
-    return false;
-  }
-
-  // TODO: Fix inlining functions from object.d. Currently errors because of
-  // TypeInfo type-mismatch issue (TypeInfo classes get special treatment by the
-  // compiler). To start working on it: comment-out this check and druntime will
-  // fail to compile.
-  if (fdecl.getModule()->ident == Id::object) {
-    IF_LOG Logger::println("Inlining of object.d functions is disabled");
-    return false;
-  }
-
-  if (fdecl.semanticRun >= PASSsemantic3) {
-    // If semantic analysis has come this far, the function will be defined
-    // elsewhere and should not get the available_externally attribute from
-    // here.
-    IF_LOG Logger::println("Semantic analysis already completed");
-    return false;
-  }
-
-  if (alreadyOrWillBeDefined(fdecl)) {
-    // This check is needed because of ICEs happening because of unclear issues
-    // upon changing the codegen order without this check.
-    IF_LOG Logger::println("Function will be defined later.");
-    return false;
-  }
-
-  // Weak-linkage functions can not be inlined.
-  if (hasWeakUDA(&fdecl)) {
-    IF_LOG Logger::println("@weak functions cannot be inlined.");
-    return false;
-  }
-
-  if (!isInlineCandidate(fdecl))
-    return false;
-
-  IF_LOG Logger::println("Potential inlining candidate");
-
-  {
-    IF_LOG Logger::println("Do semantic analysis");
-    LOG_SCOPE
-
-    // The inlining is aggressive and may give semantic errors that are
-    // forward referencing errors. Simply avoid those cases for inlining.
-    uint errors = global.startGagging();
-    global.gaggedForInlining = true;
-
-    bool semantic_error = false;
-    if (fdecl.functionSemantic3()) {
-      Module::runDeferredSemantic3();
-    } else {
-      IF_LOG Logger::println("Failed functionSemantic3.");
-      semantic_error = true;
-    }
-
-    global.gaggedForInlining = false;
-    if (global.endGagging(errors) || semantic_error) {
-      IF_LOG Logger::println("Errors occured during semantic analysis.");
-      return false;
-    }
-    assert(fdecl.semanticRun >= PASSsemantic3done);
-  }
-
-  // FuncDeclaration::naked is set by the AsmParser during semantic3 analysis,
-  // and so this check can only be done at this late point.
-  if (fdecl.naked) {
-    IF_LOG Logger::println("Naked asm functions cannot be inlined.");
-    return false;
-  }
-
-  IF_LOG Logger::println("defineAsExternallyAvailable? Yes.");
-  return true;
-}
-}
-////////////////////////////////////////////////////////////////////////////////
-
 llvm::FunctionType *DtoFunctionType(Type *type, IrFuncTy &irFty, Type *thistype,
                                     Type *nesttype, bool isMain, bool isCtor,
                                     bool isIntrinsic, bool hasSel) {
@@ -942,9 +795,9 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) {
   }
 
   if (!linkageAvailableExternally && !alreadyOrWillBeDefined(*fd)) {
-      IF_LOG Logger::println("Skipping '%s'.", fd->toPrettyChars());
+    IF_LOG Logger::println("Skipping '%s'.", fd->toPrettyChars());
-      fd->ir->setDefined();
+    fd->ir->setDefined();
-      return;
+    return;
   }
 
   DtoDeclareFunction(fd);
@@ -1067,7 +920,7 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) {
   // assert(gIR->scopes.empty());
   gIR->scopes.push_back(IRScope(beginbb));
 
-  // Set the FastMath options for this function scope.
+// Set the FastMath options for this function scope.
 #if LDC_LLVM_VER >= 308
   gIR->scopes.back().builder.setFastMathFlags(irFunc->FMF);
 #else

gen/recursivevisitor.h

@@ -29,7 +29,10 @@
 #ifndef LDC_GEN_RECURSIVEVISITOR_H
 #define LDC_GEN_RECURSIVEVISITOR_H
 
-#include "visitor.h"
+#include "ddmd/declaration.h"
+#include "ddmd/init.h"
+#include "ddmd/statement.h"
+#include "ddmd/visitor.h"
 
 class RecursiveVisitor : public Visitor {
 public:
@@ -240,40 +243,44 @@ public:
 ///////////////////////////////////////////////////////////////////////////////
 
 /// A recursive AST walker, that walks both Statements and Expressions
-/// The recursion stops at a depth where the visitor sets stop to true. Then,
+/// The recursion stops at a depth where the visitor sets stop to true.
-/// the visitor's stop is reset to false and traversal continues at the next
+/// If `continueAfterStop` is true, the visitor's stop is reset to false and
-/// node in the hierarchy at the same level as where stop was set.
+/// traversal continues at the next node in the hierarchy at the same level as
+/// where stop was set.
 class RecursiveWalker : public Visitor {
 public:
   StoppableVisitor *v;
+  bool continueAfterStop;
 
-  RecursiveWalker(StoppableVisitor *visitor) : v(visitor) {}
+  RecursiveWalker(StoppableVisitor *visitor, bool _continueAfterStop = true)
+      : v(visitor), continueAfterStop(_continueAfterStop) {}
 
   template <class T> bool recurse(T *stmt) {
     if (stmt) {
       stmt->accept(this);
     }
-    return false;
+    return v->stop;
   }
 
   template <class T> bool recurse(Array<T> *stmts) {
     if (stmts) {
       for (auto s : *stmts) {
-        recurse(s);
+        if (recurse(s))
+          break;
       }
     }
-    return false;
+    return v->stop;
   }
 
   template <class T> bool call_visitor(T *obj) {
     obj->accept(v);
-    if (v->stop) {
+    if (v->stop && continueAfterStop) {
       // Reset stop to false, so that traversion continues at neighboring node
       // in the tree.
       v->stop = false;
       return true;
     }
-    return false;
+    return v->stop;
   }
 
   using Visitor::visit;
@@ -326,9 +333,7 @@ public:
   }
 
   void visit(OnScopeStatement *stmt) override {
-    stmt->error("Internal Compiler Error: OnScopeStatement should have been "
+    call_visitor(stmt) || recurse(stmt->statement);
-                "lowered by frontend.");
-    fatal();
   }
 
   void visit(ThrowStatement *stmt) override {