Since I got LGTM from Reid on the clang patch and Duncan on the llvm patch I have committed these as r239480 (llvm) and r239481 (clang).
Thanks, Teresa On Tue, Jun 9, 2015 at 7:07 AM, Teresa Johnson <[email protected]> wrote: > cc: +cfe-dev for the Clang patch > > Responses below, new patches attached (clang bits unchanged from last time). > > On Mon, Jun 8, 2015 at 2:31 PM, Duncan P. N. Exon Smith > <[email protected]> wrote: >> bcc:llvmdev, +llvm-commits >> >>> On 2015 Jun 8, at 14:13, Teresa Johnson <[email protected]> wrote: >>> >>> New patches attached. Passes llvm/clang regression tests. PTAL >>> Thanks! >>> Teresa >>> >>> On Mon, Jun 8, 2015 at 1:39 PM, Duncan P. N. Exon Smith >>> <[email protected]> wrote: >>>> >>>>> On 2015 Jun 8, at 13:35, Eric Christopher <[email protected]> wrote: >>>>> >>>>> >>>>> >>>>> On Mon, Jun 8, 2015 at 1:33 PM Reid Kleckner <[email protected]> wrote: >>>>> On Mon, Jun 8, 2015 at 1:08 PM, Eric Christopher <[email protected]> >>>>> wrote: >>>>> I'd rather not have it be an llvm option at all. Just construct a >>>>> different set of passes... >>>>> >>>>> This would also solve the problem of needing multiple sets of options to >>>>> be passed to the builder. It'd be a bit of a change (i.e. having clang do >>>>> the pass setup), but I think it'd be worth it to have clang be able to >>>>> explicitly say which passes it wants. >>>>> >>>>> It's also a major change and would probably need to be discussed more >>>>> widely so for now I guess this is sorta ok. >>>>> >>>>> I think for now the bool on PassManagerBuilder is a good way to make >>>>> progress. Eventually, we may want to customize the LTO compilation phase >>>>> optimization more fully (use lower inlining threshold, defer >>>>> vectorization to link time), but we can cross that bridge when we get >>>>> there. >>>>> >>>>> Precisely. In case it wasn't clear I was saying this. >>>> >>>> This SGTM too. >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | [email protected] | 408-460-2413 >>> <patch.ElimAE_clang2.txt><patch.ElimAE_llvm2.txt> >> >> LLVM patch LGTM, with a few bikesheds below. >> >>> Index: include/llvm/InitializePasses.h >>> =================================================================== >>> --- include/llvm/InitializePasses.h (revision 237590) >>> +++ include/llvm/InitializePasses.h (working copy) >>> @@ -130,6 +130,7 @@ void initializeSanitizerCoverageModulePass(PassReg >>> void initializeDataFlowSanitizerPass(PassRegistry&); >>> void initializeScalarizerPass(PassRegistry&); >>> void initializeEarlyCSELegacyPassPass(PassRegistry &); >>> +void initializeElimAvailExternPass(PassRegistry&); >>> void initializeExpandISelPseudosPass(PassRegistry&); >>> void initializeFunctionAttrsPass(PassRegistry&); >>> void initializeGCMachineCodeAnalysisPass(PassRegistry&); >>> Index: include/llvm/Transforms/IPO.h >>> =================================================================== >>> --- include/llvm/Transforms/IPO.h (revision 237590) >>> +++ include/llvm/Transforms/IPO.h (working copy) >>> @@ -71,6 +71,12 @@ ModulePass *createGlobalOptimizerPass(); >>> ModulePass *createGlobalDCEPass(); >>> >>> >>> //===----------------------------------------------------------------------===// >>> +/// createElimAvailExternPass - This transform is designed to eliminate >> >> This is old-style commenting. Please skip the name of the function in >> the doxygen comment. > > Fixed. > >> >>> +/// available external globals (functions or global variables) >>> +/// >>> +ModulePass *createElimAvailExternPass(); >> >> This name isn't clear to me. "elimavailextern" is fine as command-line >> shorthand, but I think the function should be more explicit, something >> like `createEliminateAvailableExternallyPass()`. > > Went with the longer name. > >> >>> + >>> +//===----------------------------------------------------------------------===// >>> /// 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 237590) >>> +++ 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 237590) >>> +++ 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,93 @@ >>> +//===-- 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 "elimavailextern" >> >> Would this be better with hyphens ("elim-avail-extern")? I don't really >> have an opinion of which is better, just asking. > > Yes, I like that better, changed. > >> >>> + >>> +STATISTIC(NumAliases , "Number of global aliases removed"); >>> +STATISTIC(NumFunctions, "Number of functions removed"); >>> +STATISTIC(NumVariables, "Number of global variables removed"); >>> + >>> +namespace { >>> + struct ElimAvailExtern : public ModulePass { >> >> I think the class name (like the `create*()` function) should be spelled >> out more explicilty: `EliminateAvailableExternally`. > > Done. > >> >>> + static char ID; // Pass identification, replacement for typeid >>> + ElimAvailExtern() : ModulePass(ID) { >>> + initializeElimAvailExternPass(*PassRegistry::getPassRegistry()); >>> + } >>> + >>> + // run - Do the ElimAvailExtern pass on the specified module, >>> optionally >>> + // updating the specified callgraph to reflect the changes. >>> + // >>> + bool runOnModule(Module &M) override; >>> + }; >>> +} >>> + >>> +char ElimAvailExtern::ID = 0; >>> +INITIALIZE_PASS(ElimAvailExtern, "elimavailextern", >> >> If add hyphens to the `DEBUG_TYPE`, add hyphens here too. > > Done. > >> >>> + "Eliminate Available External Globals", false, false) >> >> s/External/Externally/ > > Done. > >> >>> + >>> +ModulePass *llvm::createElimAvailExternPass() { return new >>> ElimAvailExtern(); } >>> + >>> +bool ElimAvailExtern::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); >>> + 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->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); >>> + NumAliases++; >>> + } >>> + >>> + return Changed; >>> +} >>> Index: lib/Transforms/IPO/PassManagerBuilder.cpp >>> =================================================================== >>> --- lib/Transforms/IPO/PassManagerBuilder.cpp (revision 237590) >>> +++ lib/Transforms/IPO/PassManagerBuilder.cpp (working copy) >>> @@ -106,6 +106,7 @@ PassManagerBuilder::PassManagerBuilder() { >>> VerifyInput = false; >>> VerifyOutput = false; >>> MergeFunctions = false; >>> + PrepareForLTO = false; >>> } >>> >>> PassManagerBuilder::~PassManagerBuilder() { >>> @@ -407,6 +408,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(createElimAvailExternPass()); >>> + } >>> MPM.add(createGlobalDCEPass()); // Remove dead fns and >>> globals. >>> MPM.add(createConstantMergePass()); // Merge dup global constants >>> } >>> >> >> > > > > -- > Teresa Johnson | Software Engineer | [email protected] | 408-460-2413 -- Teresa Johnson | Software Engineer | [email protected] | 408-460-2413 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
