Index: llvm/trunk/lib/Transforms/IPO/Internalize.cpp =================================================================== --- llvm/trunk/lib/Transforms/IPO/Internalize.cpp +++ llvm/trunk/lib/Transforms/IPO/Internalize.cpp @@ -60,6 +60,10 @@ explicit InternalizePass(); explicit InternalizePass(ArrayRef ExportList); void LoadFile(const char *Filename); + bool maybeInternalize(GlobalValue &GV, + const std::set &ExternalComdats); + void checkComdatVisibility(GlobalValue &GV, + std::set &ExternalComdats); bool runOnModule(Module &M) override; void getAnalysisUsage(AnalysisUsage &AU) const override { @@ -105,40 +109,85 @@ } } -static bool shouldInternalize(const GlobalValue &GV, - const std::set &ExternalNames) { +static bool isExternallyVisible(const GlobalValue &GV, + const std::set &ExternalNames) { // Function must be defined here if (GV.isDeclaration()) - return false; + return true; // Available externally is really just a "declaration with a body". if (GV.hasAvailableExternallyLinkage()) - return false; + return true; // Assume that dllexported symbols are referenced elsewhere if (GV.hasDLLExportStorageClass()) - return false; - - // Already has internal linkage - if (GV.hasLocalLinkage()) - return false; + return true; // Marked to keep external? - if (ExternalNames.count(GV.getName())) - return false; + if (!GV.hasLocalLinkage() && ExternalNames.count(GV.getName())) + return true; + + return false; +} + +// Internalize GV if it is possible to do so, i.e. it is not externally visible +// and is not a member of an externally visible comdat. +bool InternalizePass::maybeInternalize( + GlobalValue &GV, const std::set &ExternalComdats) { + if (Comdat *C = GV.getComdat()) { + if (ExternalComdats.count(C)) + return false; + + // If a comdat is not externally visible we can drop it. + if (auto GO = dyn_cast(&GV)) + GO->setComdat(nullptr); + + if (GV.hasLocalLinkage()) + return false; + } else { + if (GV.hasLocalLinkage()) + return false; + if (isExternallyVisible(GV, ExternalNames)) + return false; + } + + GV.setVisibility(GlobalValue::DefaultVisibility); + GV.setLinkage(GlobalValue::InternalLinkage); return true; } +// If GV is part of a comdat and is externally visible, keep track of its +// comdat so that we don't internalize any of its members. +void InternalizePass::checkComdatVisibility( + GlobalValue &GV, std::set &ExternalComdats) { + Comdat *C = GV.getComdat(); + if (!C) + return; + + if (isExternallyVisible(GV, ExternalNames)) + ExternalComdats.insert(C); +} + bool InternalizePass::runOnModule(Module &M) { CallGraphWrapperPass *CGPass = getAnalysisIfAvailable(); CallGraph *CG = CGPass ? &CGPass->getCallGraph() : nullptr; CallGraphNode *ExternalNode = CG ? CG->getExternalCallingNode() : nullptr; - bool Changed = false; SmallPtrSet Used; collectUsedGlobalVariables(M, Used, false); + // Collect comdat visiblity information for the module. + std::set ExternalComdats; + if (!M.getComdatSymbolTable().empty()) { + for (Function &F : M) + checkComdatVisibility(F, ExternalComdats); + for (GlobalVariable &GV : M.globals()) + checkComdatVisibility(GV, ExternalComdats); + for (GlobalAlias &GA : M.aliases()) + checkComdatVisibility(GA, ExternalComdats); + } + // We must assume that globals in llvm.used have a reference that not even // the linker can see, so we don't internalize them. // For llvm.compiler.used the situation is a bit fuzzy. The assembler and @@ -154,17 +203,13 @@ // Mark all functions not in the api as internal. for (Module::iterator I = M.begin(), E = M.end(); I != E; ++I) { - if (!shouldInternalize(*I, ExternalNames)) + if (!maybeInternalize(*I, ExternalComdats)) continue; - I->setVisibility(GlobalValue::DefaultVisibility); - I->setLinkage(GlobalValue::InternalLinkage); - if (ExternalNode) // Remove a callgraph edge from the external node to this function. ExternalNode->removeOneAbstractEdgeTo((*CG)[I]); - Changed = true; ++NumFunctions; DEBUG(dbgs() << "Internalizing func " << I->getName() << "\n"); } @@ -191,12 +236,9 @@ // internal as well. for (Module::global_iterator I = M.global_begin(), E = M.global_end(); I != E; ++I) { - if (!shouldInternalize(*I, ExternalNames)) + if (!maybeInternalize(*I, ExternalComdats)) continue; - I->setVisibility(GlobalValue::DefaultVisibility); - I->setLinkage(GlobalValue::InternalLinkage); - Changed = true; ++NumGlobals; DEBUG(dbgs() << "Internalized gvar " << I->getName() << "\n"); } @@ -204,17 +246,20 @@ // Mark all aliases that are not in the api as internal as well. for (Module::alias_iterator I = M.alias_begin(), E = M.alias_end(); I != E; ++I) { - if (!shouldInternalize(*I, ExternalNames)) + if (!maybeInternalize(*I, ExternalComdats)) continue; - I->setVisibility(GlobalValue::DefaultVisibility); - I->setLinkage(GlobalValue::InternalLinkage); - Changed = true; ++NumAliases; DEBUG(dbgs() << "Internalized alias " << I->getName() << "\n"); } - return Changed; + // We do not keep track of whether this pass changed the module because + // it adds unnecessary complexity: + // 1) This pass will generally be near the start of the pass pipeline, so + // there will be no analyses to invalidate. + // 2) This pass will most likely end up changing the module and it isn't worth + // worrying about optimizing the case where the module is unchanged. + return true; } ModulePass *llvm::createInternalizePass() { return new InternalizePass(); } Index: llvm/trunk/test/Transforms/Internalize/comdat.ll =================================================================== --- llvm/trunk/test/Transforms/Internalize/comdat.ll +++ llvm/trunk/test/Transforms/Internalize/comdat.ll @@ -0,0 +1,52 @@ +; RUN: opt < %s -internalize -internalize-public-api-list c1 -internalize-public-api-list c2 -internalize-public-api-list c3 -internalize-public-api-list c4 -S | FileCheck %s + +$c1 = comdat any +$c2 = comdat any +$c3 = comdat any +$c4 = comdat any + +; CHECK: @c1_c = global i32 0, comdat($c1) +@c1_c = global i32 0, comdat($c1) + +; CHECK: @c2_b = internal global i32 0{{$}} +@c2_b = global i32 0, comdat($c2) + +; CHECK: @c3 = global i32 0, comdat{{$}} +@c3 = global i32 0, comdat + +; CHECK: @c4_a = internal global i32 0, comdat($c4) +@c4_a = internal global i32 0, comdat($c4) + +; CHECK: @c1_d = alias i32* @c1_c +@c1_d = alias i32* @c1_c + +; CHECK: @c2_c = internal alias i32* @c2_b +@c2_c = alias i32* @c2_b + +; CHECK: @c4 = alias i32* @c4_a +@c4 = alias i32* @c4_a + +; CHECK: define void @c1() comdat { +define void @c1() comdat { + ret void +} + +; CHECK: define void @c1_a() comdat($c1) { +define void @c1_a() comdat($c1) { + ret void +} + +; CHECK: define internal void @c2() { +define internal void @c2() comdat { + ret void +} + +; CHECK: define internal void @c2_a() { +define void @c2_a() comdat($c2) { + ret void +} + +; CHECK: define void @c3_a() comdat($c3) { +define void @c3_a() comdat($c3) { + ret void +}