[ThinLTO] Don't try to import noinline functions

[evgeny777]

Summary

In the attached test case attempt to import function foo causes promotion of function bar, which prevents inlining it on codegen phase.

[tejohnson]

Thanks! Couple suggestions.

lib/Analysis/ModuleSummaryAnalysis.cpp

309

Move it up here and just OR into this bool.

test/ThinLTO/X86/noinline.ll

11

It would be better to check for the non-import of foo directly. E.g. pass -save-temps to llvm-lto2, then ensure a define of foo does not exist in the llvm-dis of the resulting %t1.bc.imports file.

This would enable you to simplify the test - no need to have bar at all. Also the test will still work as expected in a possible future where we might decide to import bar as a local copy to avoid the promotion.

[evgeny777]

Addressed review comments

[tejohnson]

LGTM with one minor comment nit.
Thanks!

lib/Analysis/ModuleSummaryAnalysis.cpp

310

s/inline functions/import functions/

lib/Analysis/ModuleSummaryAnalysis.cpp

for (auto &I : F.getImportGUIDs())
     CallGraphEdges[Index.getOrInsertValueInfo(I)].updateHotness(
         CalleeInfo::HotnessType::Critical);
 
   bool NonRenamableLocal = isNonRenamableLocal(F);
   bool NotEligibleForImport =
       NonRenamableLocal || HasInlineAsmMaybeReferencingInternal ||
       // Inliner doesn't handle variadic functions.
       // FIXME: refactor this to use the same code that inliner is using.
       F.isVarArg();
   GlobalValueSummary::GVFlags Flags(F.getLinkage(), NotEligibleForImport,
                                     /* Live = */ false, F.isDSOLocal());
   FunctionSummary::FFlags FunFlags{
       F.hasFnAttribute(Attribute::ReadNone),
       F.hasFnAttribute(Attribute::ReadOnly),
       F.hasFnAttribute(Attribute::NoRecurse),
       F.returnDoesNotAlias(),
   };
   auto FuncSummary = llvm::make_unique<FunctionSummary>(
       Flags, NumInsts, FunFlags, RefEdges.takeVector(),
       CallGraphEdges.takeVector(), TypeTests.takeVector(),
       TypeTestAssumeVCalls.takeVector(), TypeCheckedLoadVCalls.takeVector(),
       TypeTestAssumeConstVCalls.takeVector(),
       TypeCheckedLoadConstVCalls.takeVector());
+  if (F.getAttributes().hasFnAttribute(Attribute::NoInline))
+    FuncSummary->setNotEligibleToImport();
   if (NonRenamableLocal)
     CantBePromoted.insert(F.getGUID());
   Index.addGlobalValueSummary(F.getName(), std::move(FuncSummary));
 }
 
 static void
 computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V,
                        DenseSet<GlobalValue::GUID> &CantBePromoted) {

test/ThinLTO/X86/Inputs/noinline.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+define i32 @foo(i32) local_unnamed_addr #0 {
+  %2 = tail call fastcc i32 @bar(i32 %0)
+  ret i32 %2
+}
+
+define internal fastcc i32 @bar(i32) unnamed_addr {
+  %2 = shl i32 %0, 1
+  %3 = add nsw i32 %2, 10
+  ret i32 %3
+}
+
+attributes #0 = { noinline }

test/ThinLTO/X86/noinline.ll

+; This test checks that ThinLTO doesn't try to import noinline function
+; which, when takes place, causes promotion of its callee.
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/noinline.ll -o %t2.bc
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t3.o \
+; RUN:   -r=%t1.bc,main,px \
+; RUN:   -r=%t1.bc,foo,    \
+; RUN:   -r=%t2.bc,foo,p
+; RUN: llvm-nm %t3.o.2 | FileCheck %s
+
+; CHECK-NOT: bar
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+; Function Attrs: nounwind ssp uwtable
+define i32 @main(i32, i8** nocapture readnone) local_unnamed_addr #0 {
+  %3 = tail call i32 @foo(i32 %0) #2
+  ret i32 %3
+}
+
+declare i32 @foo(i32) local_unnamed_addr
+
+attributes #0 = { nounwind }
+attributes #2 = { nounwind }