Fixes and improvements to cross-module inlining code.

gen/declarations.cpp

@@ -433,16 +433,16 @@ public:
       return;
     }
 
-    // FIXME: This is #673 all over again.
+    // Force codegen if this is a templated function with pragma(inline, true).
-    if (!decl->needsCodegen()) {
+    if ((decl->members->dim == 1) &&
-      // Force codegen if this is a templated function with pragma(inline, true).
+        ((*decl->members)[0]->isFuncDeclaration()) &&
-      if ((decl->members->dim == 1) &&
+        ((*decl->members)[0]->isFuncDeclaration()->inlining == PINLINEalways)) {
-          ((*decl->members)[0]->isFuncDeclaration()) &&
+      Logger::println("needsCodegen() == false, but function is marked with "
-          ((*decl->members)[0]->isFuncDeclaration()->inlining == PINLINEalways)) {
+                      "pragma(inline, true), so it really does need "
-        Logger::println("needsCodegen() == false, but function is marked with "
+                      "codegen.");
-                        "pragma(inline, true), so it really does need "
+    } else {
-                        "codegen.");
+      // FIXME: This is #673 all over again.
-      } else {
+      if (!decl->needsCodegen()) {
         Logger::println("Does not need codegen, skipping.");
         return;
       }

gen/functions.cpp

@@ -48,7 +48,7 @@
 namespace {
 
 // Use a heuristic to determine if it could make sense to inline this fdecl.
-// Note: isInlineCandidate is called _before_ full semantic analysis of fdecl.
+// Note: isInlineCandidate is called _before_ semantic3 analysis of fdecl.
 bool isInlineCandidate(FuncDeclaration &fdecl) {
   if (fdecl.inlining == PINLINEalways)
     return true;
@@ -72,9 +72,11 @@ bool alreadyOrWillBeDefined(FuncDeclaration &fdecl) {
   return true;
 }
 
-/// If true: define this function with externally_available linkage, for
+
-/// inlining potential.
+/// Returns whether `fdecl` should be emitted with externally_available
-/// If true: full semantic analysis done on this fdecl, ready for codegen.
+/// 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
@@ -87,7 +89,10 @@ bool defineAsExternallyAvailable(FuncDeclaration &fdecl) {
   }
 
   // pragma(inline, true) functions will be inlined even at -O0
-  if (!willInline() && (fdecl.inlining != PINLINEalways)) {
+  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;
   }
@@ -105,14 +110,26 @@ bool defineAsExternallyAvailable(FuncDeclaration &fdecl) {
     return false;
   }
 
-  // Disable inlining functions from object.d because of TypeInfo related
+  // TODO: Fix inlining functions from object.d. Currently errors because of
-  // issue
+  // 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;
   }
@@ -128,14 +145,6 @@ bool defineAsExternallyAvailable(FuncDeclaration &fdecl) {
 
   IF_LOG Logger::println("Potential inlining candidate");
 
-  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_LOG Logger::println("Do semantic analysis");
     LOG_SCOPE
@@ -161,12 +170,8 @@ bool defineAsExternallyAvailable(FuncDeclaration &fdecl) {
     assert(fdecl.semanticRun >= PASSsemantic3done);
   }
 
-  // For naked functions (inline assembly), we emit the assembly directly as
+  // FuncDeclaration::naked is set by the AsmParser during semantic3 analysis,
-  // globals in the text section. Emitting them during this inline pass will
+  // and so this check can only be done at this late point.
-  // therefore result in multiple definitions. Solution: don't try to inline
-  // them. These naked functions don't appear to be inlined anyway, so it is
-  // pointless at this moment to try.
-  // FuncDeclaration::naked is set by the AsmParser during semantic analysis.
   if (fdecl.naked) {
     IF_LOG Logger::println("Naked asm functions cannot be inlined.");
     return false;
@@ -596,7 +601,7 @@ void DtoDeclareFunction(FuncDeclaration *fdecl) {
   // Check if fdecl should be defined too for cross-module inlining.
   // If true, semantic is fully done for fdecl which is needed for some code
   // below (e.g. code that uses fdecl->vthis).
-  bool defineAtEnd = defineAsExternallyAvailable(*fdecl);
+  const bool defineAtEnd = defineAsExternallyAvailable(*fdecl);
   if (defineAtEnd) {
     IF_LOG Logger::println(
         "Function is an externally_available inline candidate.");
@@ -871,16 +876,16 @@ void defineParameters(IrFuncTy &irFty, VarDeclarations &parameters) {
 
 } // anonymous namespace
 
-void DtoDefineFunction(FuncDeclaration *fd, bool availableExternally) {
+void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) {
   IF_LOG Logger::println("DtoDefineFunction(%s): %s", fd->toPrettyChars(),
                          fd->loc.toChars());
   LOG_SCOPE;
-  if (availableExternally) {
+  if (linkageAvailableExternally) {
-    IF_LOG Logger::println("availableExternally = true");
+    IF_LOG Logger::println("linkageAvailableExternally = true");
   }
 
   if (fd->ir->isDefined()) {
-    if (!availableExternally &&
+    if (!linkageAvailableExternally &&
         (getIrFunc(fd)->func->getLinkage() ==
          llvm::GlobalValue::AvailableExternallyLinkage)) {
       // Fix linkage
@@ -910,7 +915,6 @@ void DtoDefineFunction(FuncDeclaration *fd, bool availableExternally) {
           fd->toPrettyChars());
     fatal();
   }
-  assert(fd->semanticRun >= PASSsemantic3done);
 
   DtoResolveFunction(fd);
 
@@ -930,12 +934,10 @@ void DtoDefineFunction(FuncDeclaration *fd, bool availableExternally) {
     return;
   }
 
-  if (!availableExternally) {
+  if (!linkageAvailableExternally && !alreadyOrWillBeDefined(*fd)) {
-    if (!alreadyOrWillBeDefined(*fd)) {
       IF_LOG Logger::println("Skipping '%s'.", fd->toPrettyChars());
       fd->ir->setDefined();
       return;
-    }
   }
 
   DtoDeclareFunction(fd);
@@ -1021,7 +1023,7 @@ void DtoDefineFunction(FuncDeclaration *fd, bool availableExternally) {
   gIR->functions.push_back(irFunc);
 
   const auto lwc = lowerFuncLinkage(fd);
-  if (availableExternally) {
+  if (linkageAvailableExternally) {
     func->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage);
     // Assert that we are not overriding a linkage type that disallows inlining
     assert(lwc.first != llvm::GlobalValue::WeakAnyLinkage &&

gen/functions.h

@@ -36,7 +36,7 @@ llvm::FunctionType *DtoFunctionType(FuncDeclaration *fdecl);
 
 void DtoResolveFunction(FuncDeclaration *fdecl);
 void DtoDeclareFunction(FuncDeclaration *fdecl);
-void DtoDefineFunction(FuncDeclaration *fd, bool availableExternally = false);
+void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally = false);
 
 void DtoDefineNakedFunction(FuncDeclaration *fd);
 void emitABIReturnAsmStmt(IRAsmBlock *asmblock, Loc &loc,

gen/optimizer.cpp

@@ -90,6 +90,14 @@ static cl::opt<cl::boolOrDefault, false, opts::FlagParser<cl::boolOrDefault>>
         cl::desc("Enable function inlining (default in -O2 and higher)"),
         cl::ZeroOrMore);
 
+static llvm::cl::opt<llvm::cl::boolOrDefault, false,
+                     opts::FlagParser<llvm::cl::boolOrDefault>>
+enableCrossModuleInlining(
+    "cross-module-inlining",
+    llvm::cl::desc("Enable cross-module function inlining (default enabled "
+                   "with inlining)"),
+    llvm::cl::ZeroOrMore, llvm::cl::Hidden);
+
 static cl::opt<bool> unitAtATime("unit-at-a-time", cl::desc("Enable basic IPO"),
                                  cl::init(true));
 
@@ -131,6 +139,11 @@ bool willInline() {
          (enableInlining == cl::BOU_UNSET && optLevel() > 1);
 }
 
+bool willCrossModuleInline() {
+  return enableCrossModuleInlining == llvm::cl::BOU_TRUE ||
+         (enableCrossModuleInlining == llvm::cl::BOU_UNSET && willInline());
+}
+
 bool isOptimizationEnabled() { return optimizeLevel != 0; }
 
 llvm::CodeGenOpt::Level codeGenOptLevel() {

gen/optimizer.h

@@ -41,6 +41,8 @@ bool ldc_optimize_module(llvm::Module *m);
 // Returns whether the normal, full inlining pass will be run.
 bool willInline();
 
+bool willCrossModuleInline();
+
 bool isOptimizationEnabled();
 
 llvm::CodeGenOpt::Level codeGenOptLevel();

tests/codegen/inlining_disablecross.d

@@ -0,0 +1,21 @@
+// Test disabling/enabling of cross-module inlining
+
+// RUN: %ldc %s -I%S -c -output-ll  -enable-cross-module-inlining -O0 -of=%t.ENA.ll && FileCheck %s --check-prefix ENABLED  < %t.ENA.ll
+// RUN: %ldc %s -I%S -c -output-ll -disable-cross-module-inlining -O3 -of=%t.DIS.ll && FileCheck %s --check-prefix DISABLED < %t.DIS.ll
+
+import inputs.inlinables;
+
+extern (C): // simplify mangling for easier matching
+
+// DISABLED-LABEL: define{{.*}} @call_easily_inlinable(
+// ENABLED-LABEL: define{{.*}} @call_easily_inlinable(
+int call_easily_inlinable(int i)
+{
+    // DISABLED: call {{.*}} @easily_inlinable(
+    return easily_inlinable(i);
+    // DISABLED: ret
+    // ENABLED: ret
+}
+
+// ENABLED-DAG: define {{.*}} @easily_inlinable(
+// DISABLED-DAG: declare {{.*}} @easily_inlinable(