[Patch] (take 2) New EliminateAvailableExternally pass / Pass down -flto
Teresa Johnson
tejohnson at google.com
Tue Jun 23 11:39:45 PDT 2015
On Thu, Jun 11, 2015 at 8:17 PM, Teresa Johnson <tejohnson at google.com>
wrote:
> Hi Nico,
>
> Sorry about that. Since I am heading out on vacation for a week tomorrow I
> went ahead and reverted for now.
>
> Teresa
>
> On Thu, Jun 11, 2015 at 6:07 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> Hi Teresa,
>>
>> this (well, 239480 really) seems to break building dynamic libraries
>> pretty decisively:
>> https://code.google.com/p/chromium/issues/detail?id=499508#c3 Can you
>> take a look, and if it takes a while to investigate, revert this for now?
>>
>> Thanks,
>> Nico
>>
>
I am back from vacation and found what was happening here. The attached
patches are largely the same as the original ones but contain a fix for
this issue (llvm patch) and a new test created from Nico's reduced test
(clang patch).
The broken code was compiled -fvisibility=hidden, and this caused the thunk
to SyncMessageFilter::Send (_ZThn16_N17SyncMessageFilter4SendEP7Message),
which is available_externally, to also be marked hidden.
With my patch, we eliminated the function's body and turned it into a
declaration, which was still marked hidden as I wasn't modifying
visibility. During AsmPrinter::doFinalization, EmitVisibility is called on
all function declarations in the module, which caused this symbol to get
hidden visibility (via a resulting call to MCSymbolELF::setVisibility). The
linker then complained because it was undefined and hidden.
Before my patch, code gen suppressed the emission of this function since it
was available externally, and as a result, EmitVisibility, and
thus MCSymbolELF::setVisibility, were simply never called. The undefined
symbol then ended up with the default visibility. It seems to me that this
essentially worked by luck.
I've fixed this by changing the visibility on globals to DefaultVisibility
when we eliminate their definitions in the new EliminateAvailableExternally
pass.
Patches attached. Tests (including the new one) all pass. Ok to commit?
Thanks,
Teresa
>> On Wed, Jun 10, 2015 at 10:49 AM, Teresa Johnson <tejohnson at google.com>
>> wrote:
>>
>>> Author: tejohnson
>>> Date: Wed Jun 10 12:49:45 2015
>>> New Revision: 239481
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=239481&view=rev
>>> Log:
>>> Pass down the -flto option to the -cc1 job, and from there into the
>>> CodeGenOptions and onto the PassManagerBuilder. This enables gating
>>> the new EliminateAvailableExternally module pass on whether we are
>>> preparing for LTO.
>>>
>>> If we are preparing for LTO (e.g. a -flto -c compile), the new pass is
>>> not
>>> included as we want to preserve available externally functions for
>>> possible
>>> link time inlining.
>>>
>>
--
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150623/bd5285b7/attachment.html>
-------------- next part --------------
Index: include/llvm/InitializePasses.h
===================================================================
--- include/llvm/InitializePasses.h (revision 239589)
+++ include/llvm/InitializePasses.h (working copy)
@@ -130,6 +130,7 @@ void initializeSanitizerCoverageModulePass(PassReg
void initializeDataFlowSanitizerPass(PassRegistry&);
void initializeScalarizerPass(PassRegistry&);
void initializeEarlyCSELegacyPassPass(PassRegistry &);
+void initializeEliminateAvailableExternallyPass(PassRegistry&);
void initializeExpandISelPseudosPass(PassRegistry&);
void initializeFunctionAttrsPass(PassRegistry&);
void initializeGCMachineCodeAnalysisPass(PassRegistry&);
Index: include/llvm/Transforms/IPO.h
===================================================================
--- include/llvm/Transforms/IPO.h (revision 239589)
+++ include/llvm/Transforms/IPO.h (working copy)
@@ -71,6 +71,12 @@ ModulePass *createGlobalOptimizerPass();
ModulePass *createGlobalDCEPass();
//===----------------------------------------------------------------------===//
+/// This transform is designed to eliminate available external globals
+/// (functions or global variables)
+///
+ModulePass *createEliminateAvailableExternallyPass();
+
+//===----------------------------------------------------------------------===//
/// createGVExtractionPass - If deleteFn is true, this pass deletes
/// the specified global values. Otherwise, it deletes as much of the module as
/// possible, except for the global values specified.
Index: include/llvm/Transforms/IPO/PassManagerBuilder.h
===================================================================
--- include/llvm/Transforms/IPO/PassManagerBuilder.h (revision 239589)
+++ include/llvm/Transforms/IPO/PassManagerBuilder.h (working copy)
@@ -121,6 +121,7 @@ class PassManagerBuilder {
bool VerifyInput;
bool VerifyOutput;
bool MergeFunctions;
+ bool PrepareForLTO;
private:
/// ExtensionList - This is list of all of the extensions that are registered.
Index: lib/Transforms/IPO/CMakeLists.txt
===================================================================
--- lib/Transforms/IPO/CMakeLists.txt (revision 239589)
+++ lib/Transforms/IPO/CMakeLists.txt (working copy)
@@ -3,6 +3,7 @@ add_llvm_library(LLVMipo
BarrierNoopPass.cpp
ConstantMerge.cpp
DeadArgumentElimination.cpp
+ ElimAvailExtern.cpp
ExtractGV.cpp
FunctionAttrs.cpp
GlobalDCE.cpp
Index: lib/Transforms/IPO/ElimAvailExtern.cpp
===================================================================
--- lib/Transforms/IPO/ElimAvailExtern.cpp (revision 0)
+++ lib/Transforms/IPO/ElimAvailExtern.cpp (working copy)
@@ -0,0 +1,99 @@
+//===-- ElimAvailExtern.cpp - DCE unreachable internal functions ----------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This transform is designed to eliminate available external global
+// definitions from the program, turning them into declarations.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/IPO.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Transforms/Utils/CtorUtils.h"
+#include "llvm/Transforms/Utils/GlobalStatus.h"
+#include "llvm/Pass.h"
+using namespace llvm;
+
+#define DEBUG_TYPE "elim-avail-extern"
+
+STATISTIC(NumAliases , "Number of global aliases removed");
+STATISTIC(NumFunctions, "Number of functions removed");
+STATISTIC(NumVariables, "Number of global variables removed");
+
+namespace {
+ struct EliminateAvailableExternally : public ModulePass {
+ static char ID; // Pass identification, replacement for typeid
+ EliminateAvailableExternally() : ModulePass(ID) {
+ initializeEliminateAvailableExternallyPass(
+ *PassRegistry::getPassRegistry());
+ }
+
+ // run - Do the EliminateAvailableExternally pass on the specified module,
+ // optionally updating the specified callgraph to reflect the changes.
+ //
+ bool runOnModule(Module &M) override;
+ };
+}
+
+char EliminateAvailableExternally::ID = 0;
+INITIALIZE_PASS(EliminateAvailableExternally, "elim-avail-extern",
+ "Eliminate Available Externally Globals", false, false)
+
+ModulePass *llvm::createEliminateAvailableExternallyPass() {
+ return new EliminateAvailableExternally();
+}
+
+bool EliminateAvailableExternally::runOnModule(Module &M) {
+ bool Changed = false;
+
+ // Drop initializers of available externally global variables.
+ for (Module::global_iterator I = M.global_begin(), E = M.global_end();
+ I != E; ++I) {
+ if (!I->hasAvailableExternallyLinkage())
+ continue;
+ if (I->hasInitializer()) {
+ Constant *Init = I->getInitializer();
+ I->setInitializer(nullptr);
+ if (isSafeToDestroyConstant(Init))
+ Init->destroyConstant();
+ }
+ I->removeDeadConstantUsers();
+ I->setLinkage(GlobalValue::ExternalLinkage);
+ I->setVisibility(GlobalValue::DefaultVisibility);
+ NumVariables++;
+ }
+
+ // Drop the bodies of available externally functions.
+ for (Module::iterator I = M.begin(), E = M.end(); I != E; ++I) {
+ if (!I->hasAvailableExternallyLinkage())
+ continue;
+ if (!I->isDeclaration())
+ // This will set the linkage to external
+ I->deleteBody();
+ I->setVisibility(GlobalValue::DefaultVisibility);
+ I->removeDeadConstantUsers();
+ NumFunctions++;
+ }
+
+ // Drop targets of available externally aliases.
+ for (Module::alias_iterator I = M.alias_begin(), E = M.alias_end(); I != E;
+ ++I) {
+ if (!I->hasAvailableExternallyLinkage())
+ continue;
+ I->setAliasee(nullptr);
+ I->removeDeadConstantUsers();
+ I->setLinkage(GlobalValue::ExternalLinkage);
+ I->setVisibility(GlobalValue::DefaultVisibility);
+ NumAliases++;
+ }
+
+ return Changed;
+}
Index: lib/Transforms/IPO/PassManagerBuilder.cpp
===================================================================
--- lib/Transforms/IPO/PassManagerBuilder.cpp (revision 239589)
+++ lib/Transforms/IPO/PassManagerBuilder.cpp (working copy)
@@ -105,6 +105,7 @@ PassManagerBuilder::PassManagerBuilder() {
VerifyInput = false;
VerifyOutput = false;
MergeFunctions = false;
+ PrepareForLTO = false;
}
PassManagerBuilder::~PassManagerBuilder() {
@@ -401,6 +402,17 @@ void PassManagerBuilder::populateModulePassManager
// GlobalOpt already deletes dead functions and globals, at -O2 try a
// late pass of GlobalDCE. It is capable of deleting dead cycles.
if (OptLevel > 1) {
+ if (!PrepareForLTO) {
+ // Remove avail extern fns and globals definitions if we aren't
+ // compiling an object file for later LTO. For LTO we want to preserve
+ // these so they are eligible for inlining at link-time. Note if they
+ // are unreferenced they will be removed by GlobalDCE below, so
+ // this only impacts referenced available externally globals.
+ // Eventually they will be suppressed during codegen, but eliminating
+ // here enables more opportunity for GlobalDCE as it may make
+ // globals referenced by available external functions dead.
+ MPM.add(createEliminateAvailableExternallyPass());
+ }
MPM.add(createGlobalDCEPass()); // Remove dead fns and globals.
MPM.add(createConstantMergePass()); // Merge dup global constants
}
-------------- next part --------------
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td (revision 239588)
+++ include/clang/Driver/Options.td (working copy)
@@ -636,7 +636,7 @@ def flat__namespace : Flag<["-"], "flat_namespace"
def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group<f_Group>;
def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, Group<f_Group>;
def flto_EQ : Joined<["-"], "flto=">, Group<clang_ignored_gcc_optimization_f_Group>;
-def flto : Flag<["-"], "flto">, Group<f_Group>;
+def flto : Flag<["-"], "flto">, Flags<[CC1Option]>, Group<f_Group>;
def fno_lto : Flag<["-"], "fno-lto">, Group<f_Group>;
def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
Group<f_Group>, Flags<[DriverOption, CoreOption]>;
Index: include/clang/Frontend/CodeGenOptions.def
===================================================================
--- include/clang/Frontend/CodeGenOptions.def (revision 239588)
+++ include/clang/Frontend/CodeGenOptions.def (working copy)
@@ -67,6 +67,8 @@ CODEGENOPT(InstrumentFunctions , 1, 0) ///< Set wh
CODEGENOPT(InstrumentForProfiling , 1, 0) ///< Set when -pg is enabled.
CODEGENOPT(LessPreciseFPMAD , 1, 0) ///< Enable less precise MAD instructions to
///< be generated.
+CODEGENOPT(PrepareForLTO , 1, 0) ///< Set when -flto is enabled on the
+ ///< compile step.
CODEGENOPT(MergeAllConstants , 1, 1) ///< Merge identical constants.
CODEGENOPT(MergeFunctions , 1, 0) ///< Set when -fmerge-functions is enabled.
CODEGENOPT(MSVolatile , 1, 0) ///< Set when /volatile:ms is enabled.
Index: lib/CodeGen/BackendUtil.cpp
===================================================================
--- lib/CodeGen/BackendUtil.cpp (revision 239588)
+++ lib/CodeGen/BackendUtil.cpp (working copy)
@@ -286,6 +286,7 @@ void EmitAssemblyHelper::CreatePasses() {
PMBuilder.DisableUnitAtATime = !CodeGenOpts.UnitAtATime;
PMBuilder.DisableUnrollLoops = !CodeGenOpts.UnrollLoops;
PMBuilder.MergeFunctions = CodeGenOpts.MergeFunctions;
+ PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;
PMBuilder.RerollLoops = CodeGenOpts.RerollLoops;
PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp (revision 239588)
+++ lib/Driver/Tools.cpp (working copy)
@@ -2834,6 +2834,10 @@ void Clang::ConstructJob(Compilation &C, const Job
assert((isa<CompileJobAction>(JA) || isa<BackendJobAction>(JA)) &&
"Invalid action for clang tool.");
+ if (JA.getType() == types::TY_LTO_IR ||
+ JA.getType() == types::TY_LTO_BC) {
+ CmdArgs.push_back("-flto");
+ }
if (JA.getType() == types::TY_Nothing) {
CmdArgs.push_back("-fsyntax-only");
} else if (JA.getType() == types::TY_LLVM_IR ||
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp (revision 239588)
+++ lib/Frontend/CompilerInvocation.cpp (working copy)
@@ -485,6 +485,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts,
Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
+ Opts.PrepareForLTO = Args.hasArg(OPT_flto);
+
Opts.MSVolatile = Args.hasArg(OPT_fms_volatile);
Opts.VectorizeBB = Args.hasArg(OPT_vectorize_slp_aggressive);
Index: test/CodeGen/available-externally-hidden.cpp
===================================================================
--- test/CodeGen/available-externally-hidden.cpp (revision 0)
+++ test/CodeGen/available-externally-hidden.cpp (working copy)
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -O2 -fvisibility hidden -std=c++11 -emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s
+
+// Ensure that available_externally functions eliminated at -O2 are now
+// declarations, and are not emitted as hidden with -fvisibility=hidden,
+// but rather with default visibility.
+struct Filter {
+ virtual void Foo();
+ int a;
+};
+
+class Message{};
+class Sender {
+ public:
+ virtual bool Send(Message* msg) = 0;
+
+ protected:
+ virtual ~Sender() {}
+};
+
+// CHECK: declare zeroext i1 @_ZThn16_N17SyncMessageFilter4SendEP7Message
+class SyncMessageFilter : public Filter, public Sender {
+ public:
+ bool Send(Message* message) override;
+};
+
+class TestSyncMessageFilter : public SyncMessageFilter {
+};
+
+int main() {
+TestSyncMessageFilter* f = new TestSyncMessageFilter;
+ f->Send(new Message);
+}
Index: test/CodeGen/available-externally-suppress.c
===================================================================
--- test/CodeGen/available-externally-suppress.c (revision 239588)
+++ test/CodeGen/available-externally-suppress.c (working copy)
@@ -1,6 +1,10 @@
// RUN: %clang_cc1 -emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -O2 -fno-inline -emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -flto -O2 -fno-inline -emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s -check-prefix=LTO
// Ensure that we don't emit available_externally functions at -O0.
+// Also should not emit them at -O2, unless -flto is present in which case
+// we should preserve them for link-time inlining decisions.
int x;
inline void f0(int y) { x = y; }
@@ -7,6 +11,8 @@ inline void f0(int y) { x = y; }
// CHECK-LABEL: define void @test()
// CHECK: declare void @f0(i32)
+// LTO-LABEL: define void @test()
+// LTO: define available_externally void @f0
void test() {
f0(17);
}
@@ -19,9 +25,13 @@ inline int __attribute__((always_inline)) f1(int x
}
// CHECK: @test1
+// LTO: @test1
int test1(int x) {
// CHECK: br i1
// CHECK-NOT: call {{.*}} @f1
// CHECK: ret i32
+ // LTO: br i1
+ // LTO-NOT: call {{.*}} @f1
+ // LTO: ret i32
return f1(x);
}
More information about the cfe-commits
mailing list