[llvm] r278432 - Don't import variadic functions

Piotr Padlewski via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 11 15:13:57 PDT 2016


Author: prazek
Date: Thu Aug 11 17:13:57 2016
New Revision: 278432

URL: http://llvm.org/viewvc/llvm-project?rev=278432&view=rev
Log:
Don't import variadic functions

Summary:
This patch adds IsVariadicFunction bit to summary in order
to not import variadic functions. Inliner doesn't inline
variadic functions because it is hard to reason about it.

This one small fix improves Importer by about 16%
(going from 86% to 100% of imported functions that are
inlined anywhere)
on some spec benchmarks like 'int' and others.

Reviewers: eraman, mehdi_amini, tejohnson

Subscribers: mehdi_amini, llvm-commits

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

Modified:
    llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
    llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
    llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
    llvm/trunk/test/Bitcode/thinlto-function-summary.ll
    llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport.ll
    llvm/trunk/test/Transforms/FunctionImport/funcimport.ll

Modified: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h?rev=278432&r1=278431&r2=278432&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h (original)
+++ llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h Thu Aug 11 17:13:57 2016
@@ -104,11 +104,23 @@ public:
     /// Indicate if the global value is located in a specific section.
     unsigned HasSection : 1;
 
+    /// Indicate if the function is not viable to inline.
+    unsigned IsNotViableToInline : 1;
+
     /// Convenience Constructors
-    explicit GVFlags(GlobalValue::LinkageTypes Linkage, bool HasSection)
-        : Linkage(Linkage), HasSection(HasSection) {}
+    explicit GVFlags(GlobalValue::LinkageTypes Linkage, bool HasSection,
+                     bool IsNotViableToInline)
+        : Linkage(Linkage), HasSection(HasSection),
+          IsNotViableToInline(IsNotViableToInline) {}
+
     GVFlags(const GlobalValue &GV)
-        : Linkage(GV.getLinkage()), HasSection(GV.hasSection()) {}
+        : Linkage(GV.getLinkage()), HasSection(GV.hasSection()) {
+      IsNotViableToInline = false;
+      if (const auto *F = dyn_cast<Function>(&GV))
+        // Inliner doesn't handle variadic functions.
+        // FIXME: refactor this to use the same code that inliner is using.
+        IsNotViableToInline = F->isVarArg();
+    }
   };
 
 private:
@@ -175,6 +187,8 @@ public:
     Flags.Linkage = Linkage;
   }
 
+  bool isNotViableToInline() const { return Flags.IsNotViableToInline; }
+
   /// Return true if this summary is for a GlobalValue that needs promotion
   /// to be referenced from another module.
   bool needsRenaming() const { return GlobalValue::isLocalLinkage(linkage()); }

Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=278432&r1=278431&r2=278432&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Thu Aug 11 17:13:57 2016
@@ -720,7 +720,7 @@ static GlobalValue::LinkageTypes getDeco
   }
 }
 
-// Decode the flags for GlobalValue in the summary
+/// Decode the flags for GlobalValue in the summary.
 static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags,
                                                             uint64_t Version) {
   // Summary were not emitted before LLVM 3.9, we don't need to upgrade Linkage
@@ -728,8 +728,9 @@ static GlobalValueSummary::GVFlags getDe
   // to getDecodedLinkage() will need to be taken into account here as above.
   auto Linkage = GlobalValue::LinkageTypes(RawFlags & 0xF); // 4 bits
   RawFlags = RawFlags >> 4;
-  auto HasSection = RawFlags & 0x1; // bool
-  return GlobalValueSummary::GVFlags(Linkage, HasSection);
+  bool HasSection = RawFlags & 0x1;
+  bool IsNotViableToInline = RawFlags & 0x2;
+  return GlobalValueSummary::GVFlags(Linkage, HasSection, IsNotViableToInline);
 }
 
 static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) {

Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=278432&r1=278431&r2=278432&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Thu Aug 11 17:13:57 2016
@@ -991,7 +991,7 @@ static uint64_t getEncodedGVSummaryFlags
   uint64_t RawFlags = 0;
 
   RawFlags |= Flags.HasSection; // bool
-
+  RawFlags |= (Flags.IsNotViableToInline << 1);
   // Linkage don't need to be remapped at that time for the summary. Any future
   // change to the getEncodedLinkage() function will need to be taken into
   // account here as well.

Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=278432&r1=278431&r2=278432&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Thu Aug 11 17:13:57 2016
@@ -130,6 +130,10 @@ static bool eligibleForImport(const Modu
     // FIXME: we may be able to import it by copying it without promotion.
     return false;
 
+  // Don't import functions that are not viable to inline.
+  if (Summary.isNotViableToInline())
+    return false;
+
   // Check references (and potential calls) in the same module. If the current
   // value references a global that can't be externally referenced it is not
   // eligible for import.

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=278432&r1=278431&r2=278432&view=diff
==============================================================================
--- llvm/trunk/test/Bitcode/thinlto-function-summary.ll (original)
+++ llvm/trunk/test/Bitcode/thinlto-function-summary.ll Thu Aug 11 17:13:57 2016
@@ -9,13 +9,17 @@
 ; BC-NEXT: <PERMODULE {{.*}} op0=1 op1=0
 ; BC-NEXT: <PERMODULE {{.*}} op0=2 op1=0
 ; BC-NEXT: <PERMODULE {{.*}} op0=3 op1=7
-; BC-NEXT: <ALIAS {{.*}} op0=4 op1=0 op2=3
+; BC-NEXT: <PERMODULE {{.*}} op0=4 op1=32
+; BC-NEXT: <ALIAS {{.*}} op0=5 op1=0 op2=3
 ; BC-NEXT: </GLOBALVAL_SUMMARY_BLOCK
 ; BC-NEXT: <VALUE_SYMTAB
-; BC-NEXT: <FNENTRY {{.*}} op0=3 {{.*}}> record string = 'anon.
+; BC-NEXT: <FNENTRY {{.*}} op0=4 {{.*}}> record string = 'variadic'
 ; BC-NEXT: <FNENTRY {{.*}} op0=1 {{.*}}> record string = 'foo'
 ; BC-NEXT: <FNENTRY {{.*}} op0=2 {{.*}}> record string = 'bar'
-; BC-NEXT: <FNENTRY {{.*}} op0=4 {{.*}}> record string = 'f'
+; BC-NEXT: <FNENTRY {{.*}} op0=5 {{.*}}> record string = 'f'
+; BC-NEXT: <ENTRY {{.*}} record string = 'h'
+; BC-NEXT: <FNENTRY {{.*}} op0=3 {{.*}}> record string = 'anon.
+
 
 ; RUN: opt -name-anon-functions -module-summary < %s | llvm-dis | FileCheck %s
 ; Check that this round-trips correctly.
@@ -56,3 +60,7 @@ entry:
 return:         ; preds = %entry
         ret void
 }
+
+define i32 @variadic(...) {
+    ret i32 42
+}

Modified: llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport.ll?rev=278432&r1=278431&r2=278432&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport.ll (original)
+++ llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport.ll Thu Aug 11 17:13:57 2016
@@ -11,6 +11,7 @@
 define void @globalfunc1() #0 {
 entry:
   call void @funcwithpersonality()
+  call void (...) @variadic()
   ret void
 }
 
@@ -146,4 +147,8 @@ entry:
   ret void
 }
 
+; Variadic function should not be imported because inliner doesn't handle it.
+define void @variadic(...) {
+    ret void
+}
 

Modified: llvm/trunk/test/Transforms/FunctionImport/funcimport.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/funcimport.ll?rev=278432&r1=278431&r2=278432&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/FunctionImport/funcimport.ll (original)
+++ llvm/trunk/test/Transforms/FunctionImport/funcimport.ll Thu Aug 11 17:13:57 2016
@@ -32,6 +32,7 @@ entry:
   call void (...) @weakfunc()
   call void (...) @linkoncefunc2()
   call void (...) @referencelargelinkonce()
+  call void (...) @variadic()
   ret i32 0
 }
 
@@ -105,6 +106,9 @@ declare void @linkoncefunc2(...) #1
 ; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.{{.*}}() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) !thinlto_src_module !0 {
 ; INSTLIM5-DAG: declare hidden void @funcwithpersonality.llvm.{{.*}}()
 
+; CHECK-DAG: declare void @variadic(...)
+declare void @variadic(...)
+
 ; INSTLIMDEF-DAG: Import globalfunc2
 ; INSTLIMDEF-DAG: 13 function-import - Number of functions imported
 ; CHECK-DAG: !0 = !{!"{{.*}}/Inputs/funcimport.ll"}




More information about the llvm-commits mailing list