[llvm] r348068 - [ThinLTO] Allow importing of functions with var args

[Teresa Johnson via llvm-commits]

Author: tejohnson
Date: Fri Nov 30 21:11:46 2018
New Revision: 348068

URL: http://llvm.org/viewvc/llvm-project?rev=348068&view=rev
Log:
[ThinLTO] Allow importing of functions with var args

Summary:
Follow up to D54270, which allowed importing of var args functions
unless they called va_start. As pointed out in the post-commit comments
on that patch, the inliner can handle functions that call va_start in
certain situations as well. Go ahead and enable importing of all var
args functions. Measurements on a large binary show that this increases
imports and binary size by an insignificant amount.

Reviewers: davidxl

Subscribers: mehdi_amini, inglorion, eraman, steven_wu, dexonsmith, llvm-commits

Differential Revision: https://reviews.llvm.org/D54607

Modified:
    llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
    llvm/trunk/test/Bitcode/thinlto-function-summary.ll
    llvm/trunk/test/Transforms/FunctionImport/funcimport.ll

Modified: llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp?rev=348068&r1=348067&r2=348068&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp Fri Nov 30 21:11:46 2018
@@ -256,15 +256,10 @@ static void computeFunctionSummary(Modul
   std::vector<const Instruction *> NonVolatileLoads;
 
   bool HasInlineAsmMaybeReferencingInternal = false;
-  bool InitsVarArgs = false;
   for (const BasicBlock &BB : F)
     for (const Instruction &I : BB) {
       if (isa<DbgInfoIntrinsic>(I))
         continue;
-      if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(&I)) {
-        if (II->getIntrinsicID() == Intrinsic::vastart)
-          InitsVarArgs = true;
-      }
       ++NumInsts;
       if (isNonVolatileLoad(&I)) {
         // Postpone processing of non-volatile load instructions
@@ -397,11 +392,9 @@ static void computeFunctionSummary(Modul
       F.hasFnAttribute(Attribute::ReadNone),
       F.hasFnAttribute(Attribute::ReadOnly),
       F.hasFnAttribute(Attribute::NoRecurse), F.returnDoesNotAlias(),
-      // Inliner doesn't handle variadic functions with va_start calls.
       // FIXME: refactor this to use the same code that inliner is using.
-      InitsVarArgs ||
-          // Don't try to import functions with noinline attribute.
-          F.getAttributes().hasFnAttribute(Attribute::NoInline)};
+      // Don't try to import functions with noinline attribute.
+      F.getAttributes().hasFnAttribute(Attribute::NoInline)};
   auto FuncSummary = llvm::make_unique<FunctionSummary>(
       Flags, NumInsts, FunFlags, std::move(Refs), CallGraphEdges.takeVector(),
       TypeTests.takeVector(), TypeTestAssumeVCalls.takeVector(),

Modified: llvm/trunk/test/Bitcode/thinlto-function-summary.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/thinlto-function-summary.ll?rev=348068&r1=348067&r2=348068&view=diff
==============================================================================
--- llvm/trunk/test/Bitcode/thinlto-function-summary.ll (original)
+++ llvm/trunk/test/Bitcode/thinlto-function-summary.ll Fri Nov 30 21:11:46 2018
@@ -22,10 +22,7 @@
 ; BC-NEXT: <PERMODULE {{.*}} op0=1 op1=0
 ; BC-NEXT: <PERMODULE {{.*}} op0=2 op1=0
 ; BC-NEXT: <PERMODULE {{.*}} op0=3 op1=7
-; Summary for @variadic has flags (op3) = 16 since non-inlinable owing to
-; va_start call.
-; flag is set due to va_start call.
-; BC-NEXT: <PERMODULE {{.*}} op0=4 op1=0 op2=4 op3=16
+; BC-NEXT: <PERMODULE {{.*}} op0=4 op1=0 op2=4 op3=0
 ; BC-NEXT: <ALIAS {{.*}} op0=6 op1=0 op2=3
 ; BC-NEXT: </GLOBALVAL_SUMMARY_BLOCK
 ; BC: <STRTAB_BLOCK

Modified: llvm/trunk/test/Transforms/FunctionImport/funcimport.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/funcimport.ll?rev=348068&r1=348067&r2=348068&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/FunctionImport/funcimport.ll (original)
+++ llvm/trunk/test/Transforms/FunctionImport/funcimport.ll Fri Nov 30 21:11:46 2018
@@ -110,13 +110,13 @@ declare void @linkoncefunc2(...) #1
 ; CHECK-DAG: define available_externally void @variadic_no_va_start(...) !thinlto_src_module !0 {
 declare void @variadic_no_va_start(...)
 
-; We shouldn't import variadic functions without a va_start, since the inliner
-; cannot handle them.
-; CHECK-DAG: declare void @variadic_va_start(...)
+; We can import variadic functions with a va_start, since the inliner
+; can sometimes handle them.
+; CHECK-DAG: define available_externally void @variadic_va_start(...)
 declare void @variadic_va_start(...)
 
 ; INSTLIMDEF-DAG: Import globalfunc2
-; INSTLIMDEF-DAG: 14 function-import - Number of functions imported
+; INSTLIMDEF-DAG: 15 function-import - Number of functions imported
 ; INSTLIMDEF-DAG: 4 function-import - Number of global variables imported
 
 ; CHECK-DAG: !0 = !{!"{{.*}}/Inputs/funcimport.ll"}
@@ -156,7 +156,7 @@ declare void @variadic_va_start(...)
 ; GUID-DAG: GUID {{.*}} is linkoncefunc
 
 ; DUMP:       Module [[M1:.*]] imports from 1 module
-; DUMP-NEXT:  14 functions imported from [[M2:.*]]
+; DUMP-NEXT:  15 functions imported from [[M2:.*]]
 ; DUMP-NEXT:  4 vars imported from [[M2]]
-; DUMP:       Imported 14 functions for Module [[M1]]
+; DUMP:       Imported 15 functions for Module [[M1]]
 ; DUMP-NEXT:  Imported 4 global variables for Module [[M1]]