Skip to content

Commit 332b3b2

Browse files
prazekprazek
committedAug 11, 2016
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 llvm-svn: 278432
1 parent 6daefcf commit 332b3b2

File tree

7 files changed

+46
-10
lines changed

7 files changed

+46
-10
lines changed
 

‎llvm/include/llvm/IR/ModuleSummaryIndex.h

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,23 @@ class GlobalValueSummary {
104104
/// Indicate if the global value is located in a specific section.
105105
unsigned HasSection : 1;
106106

107+
/// Indicate if the function is not viable to inline.
108+
unsigned IsNotViableToInline : 1;
109+
107110
/// Convenience Constructors
108-
explicit GVFlags(GlobalValue::LinkageTypes Linkage, bool HasSection)
109-
: Linkage(Linkage), HasSection(HasSection) {}
111+
explicit GVFlags(GlobalValue::LinkageTypes Linkage, bool HasSection,
112+
bool IsNotViableToInline)
113+
: Linkage(Linkage), HasSection(HasSection),
114+
IsNotViableToInline(IsNotViableToInline) {}
115+
110116
GVFlags(const GlobalValue &GV)
111-
: Linkage(GV.getLinkage()), HasSection(GV.hasSection()) {}
117+
: Linkage(GV.getLinkage()), HasSection(GV.hasSection()) {
118+
IsNotViableToInline = false;
119+
if (const auto *F = dyn_cast<Function>(&GV))
120+
// Inliner doesn't handle variadic functions.
121+
// FIXME: refactor this to use the same code that inliner is using.
122+
IsNotViableToInline = F->isVarArg();
123+
}
112124
};
113125

114126
private:
@@ -175,6 +187,8 @@ class GlobalValueSummary {
175187
Flags.Linkage = Linkage;
176188
}
177189

190+
bool isNotViableToInline() const { return Flags.IsNotViableToInline; }
191+
178192
/// Return true if this summary is for a GlobalValue that needs promotion
179193
/// to be referenced from another module.
180194
bool needsRenaming() const { return GlobalValue::isLocalLinkage(linkage()); }

‎llvm/lib/Bitcode/Reader/BitcodeReader.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -720,16 +720,17 @@ static GlobalValue::LinkageTypes getDecodedLinkage(unsigned Val) {
720720
}
721721
}
722722

723-
// Decode the flags for GlobalValue in the summary
723+
/// Decode the flags for GlobalValue in the summary.
724724
static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags,
725725
uint64_t Version) {
726726
// Summary were not emitted before LLVM 3.9, we don't need to upgrade Linkage
727727
// like getDecodedLinkage() above. Any future change to the linkage enum and
728728
// to getDecodedLinkage() will need to be taken into account here as above.
729729
auto Linkage = GlobalValue::LinkageTypes(RawFlags & 0xF); // 4 bits
730730
RawFlags = RawFlags >> 4;
731-
auto HasSection = RawFlags & 0x1; // bool
732-
return GlobalValueSummary::GVFlags(Linkage, HasSection);
731+
bool HasSection = RawFlags & 0x1;
732+
bool IsNotViableToInline = RawFlags & 0x2;
733+
return GlobalValueSummary::GVFlags(Linkage, HasSection, IsNotViableToInline);
733734
}
734735

735736
static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) {

‎llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -991,7 +991,7 @@ static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
991991
uint64_t RawFlags = 0;
992992

993993
RawFlags |= Flags.HasSection; // bool
994-
994+
RawFlags |= (Flags.IsNotViableToInline << 1);
995995
// Linkage don't need to be remapped at that time for the summary. Any future
996996
// change to the getEncodedLinkage() function will need to be taken into
997997
// account here as well.

‎llvm/lib/Transforms/IPO/FunctionImport.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ static bool eligibleForImport(const ModuleSummaryIndex &Index,
130130
// FIXME: we may be able to import it by copying it without promotion.
131131
return false;
132132

133+
// Don't import functions that are not viable to inline.
134+
if (Summary.isNotViableToInline())
135+
return false;
136+
133137
// Check references (and potential calls) in the same module. If the current
134138
// value references a global that can't be externally referenced it is not
135139
// eligible for import.

‎llvm/test/Bitcode/thinlto-function-summary.ll

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,17 @@
99
; BC-NEXT: <PERMODULE {{.*}} op0=1 op1=0
1010
; BC-NEXT: <PERMODULE {{.*}} op0=2 op1=0
1111
; BC-NEXT: <PERMODULE {{.*}} op0=3 op1=7
12-
; BC-NEXT: <ALIAS {{.*}} op0=4 op1=0 op2=3
12+
; BC-NEXT: <PERMODULE {{.*}} op0=4 op1=32
13+
; BC-NEXT: <ALIAS {{.*}} op0=5 op1=0 op2=3
1314
; BC-NEXT: </GLOBALVAL_SUMMARY_BLOCK
1415
; BC-NEXT: <VALUE_SYMTAB
15-
; BC-NEXT: <FNENTRY {{.*}} op0=3 {{.*}}> record string = 'anon.
16+
; BC-NEXT: <FNENTRY {{.*}} op0=4 {{.*}}> record string = 'variadic'
1617
; BC-NEXT: <FNENTRY {{.*}} op0=1 {{.*}}> record string = 'foo'
1718
; BC-NEXT: <FNENTRY {{.*}} op0=2 {{.*}}> record string = 'bar'
18-
; BC-NEXT: <FNENTRY {{.*}} op0=4 {{.*}}> record string = 'f'
19+
; BC-NEXT: <FNENTRY {{.*}} op0=5 {{.*}}> record string = 'f'
20+
; BC-NEXT: <ENTRY {{.*}} record string = 'h'
21+
; BC-NEXT: <FNENTRY {{.*}} op0=3 {{.*}}> record string = 'anon.
22+
1923

2024
; RUN: opt -name-anon-functions -module-summary < %s | llvm-dis | FileCheck %s
2125
; Check that this round-trips correctly.
@@ -56,3 +60,7 @@ entry:
5660
return: ; preds = %entry
5761
ret void
5862
}
63+
64+
define i32 @variadic(...) {
65+
ret i32 42
66+
}

‎llvm/test/Transforms/FunctionImport/Inputs/funcimport.ll

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
define void @globalfunc1() #0 {
1212
entry:
1313
call void @funcwithpersonality()
14+
call void (...) @variadic()
1415
ret void
1516
}
1617

@@ -146,4 +147,8 @@ entry:
146147
ret void
147148
}
148149

150+
; Variadic function should not be imported because inliner doesn't handle it.
151+
define void @variadic(...) {
152+
ret void
153+
}
149154

‎llvm/test/Transforms/FunctionImport/funcimport.ll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ entry:
3232
call void (...) @weakfunc()
3333
call void (...) @linkoncefunc2()
3434
call void (...) @referencelargelinkonce()
35+
call void (...) @variadic()
3536
ret i32 0
3637
}
3738

@@ -105,6 +106,9 @@ declare void @linkoncefunc2(...) #1
105106
; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.{{.*}}() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) !thinlto_src_module !0 {
106107
; INSTLIM5-DAG: declare hidden void @funcwithpersonality.llvm.{{.*}}()
107108

109+
; CHECK-DAG: declare void @variadic(...)
110+
declare void @variadic(...)
111+
108112
; INSTLIMDEF-DAG: Import globalfunc2
109113
; INSTLIMDEF-DAG: 13 function-import - Number of functions imported
110114
; CHECK-DAG: !0 = !{!"{{.*}}/Inputs/funcimport.ll"}

0 commit comments

Comments
 (0)