[llvm] r278432 - Don't import variadic functions
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 11 21:39:59 PDT 2016
> On Aug 11, 2016, at 3:13 PM, Piotr Padlewski via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> 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.
The description is a bit out-of-sync with the content.
—
Mehdi
>
> 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"}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list