Index: llvm/trunk/include/llvm/LTO/LTO.h =================================================================== --- llvm/trunk/include/llvm/LTO/LTO.h +++ llvm/trunk/include/llvm/LTO/LTO.h @@ -306,6 +306,11 @@ struct RegularLTOState { RegularLTOState(unsigned ParallelCodeGenParallelismLevel, Config &Conf); + struct CommonResolution { + uint64_t Size = 0; + unsigned Align = 0; + }; + std::map Commons; unsigned ParallelCodeGenParallelismLevel; LTOLLVMContext Ctx; Index: llvm/trunk/lib/LTO/LTO.cpp =================================================================== --- llvm/trunk/lib/LTO/LTO.cpp +++ llvm/trunk/lib/LTO/LTO.cpp @@ -292,6 +292,14 @@ break; } } + // Common resolution: collect the maximum size/alignment. + // FIXME: right now we ignore the prevailing information, it is not clear + // what is the "right" behavior here. + if (Sym.getFlags() & object::BasicSymbolRef::SF_Common) { + auto &CommonRes = RegularLTO.Commons[Sym.getIRName()]; + CommonRes.Size = std::max(CommonRes.Size, Sym.getCommonSize()); + CommonRes.Align = std::max(CommonRes.Align, Sym.getCommonAlignment()); + } // FIXME: use proposed local attribute for FinalDefinitionInLinkageUnit. } @@ -361,6 +369,31 @@ } Error LTO::runRegularLTO(AddOutputFn AddOutput) { + // Make sure commons have the right size/alignment: we kept the largest from + // all the prevailing when adding the inputs, and we apply it here. + for (auto &I : RegularLTO.Commons) { + ArrayType *Ty = + ArrayType::get(Type::getInt8Ty(RegularLTO.Ctx), I.second.Size); + GlobalVariable *OldGV = RegularLTO.CombinedModule->getNamedGlobal(I.first); + if (OldGV && OldGV->getType()->getElementType() == Ty) { + // Don't create a new global if the type is already correct, just make + // sure the alignment is correct. + OldGV->setAlignment(I.second.Align); + continue; + } + auto *GV = new GlobalVariable(*RegularLTO.CombinedModule, Ty, false, + GlobalValue::CommonLinkage, + ConstantAggregateZero::get(Ty), ""); + GV->setAlignment(I.second.Align); + if (OldGV) { + OldGV->replaceAllUsesWith(ConstantExpr::getBitCast(GV, OldGV->getType())); + GV->takeName(OldGV); + OldGV->eraseFromParent(); + } else { + GV->setName(I.first); + } + } + if (Conf.PreOptModuleHook && !Conf.PreOptModuleHook(0, *RegularLTO.CombinedModule)) return Error(); Index: llvm/trunk/test/tools/llvm-lto2/Inputs/common.ll =================================================================== --- llvm/trunk/test/tools/llvm-lto2/Inputs/common.ll +++ llvm/trunk/test/tools/llvm-lto2/Inputs/common.ll @@ -0,0 +1,7 @@ +target triple = "x86_64-apple-macosx10.11.0" + +@v = common global i16 0, align 4 + +define i16 *@bar() { + ret i16 *@v +} \ No newline at end of file Index: llvm/trunk/test/tools/llvm-lto2/common.ll =================================================================== --- llvm/trunk/test/tools/llvm-lto2/common.ll +++ llvm/trunk/test/tools/llvm-lto2/common.ll @@ -0,0 +1,65 @@ +; RUN: llvm-as < %s > %t1.bc +; RUN: llvm-as < %p/Inputs/common.ll > %t2.bc + +; Test that the common merging (size + alignment) is properly handled + +; Client marked the "large with little alignment" one as prevailing +; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \ +; RUN: -r %t1.bc,v,x \ +; RUN: -r %t2.bc,v,px \ +; RUN: -r %t1.bc,foo,px \ +; RUN: -r %t2.bc,bar,px +; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s + +; Same as before, but reversing the order of the inputs +; RUN: llvm-lto2 %t2.bc %t1.bc -o %t.o -save-temps \ +; RUN: -r %t1.bc,v,x \ +; RUN: -r %t2.bc,v,px \ +; RUN: -r %t1.bc,foo,px \ +; RUN: -r %t2.bc,bar,px +; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s + + +; Client marked the "small with large alignment" one as prevailing +; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \ +; RUN: -r %t1.bc,v,px \ +; RUN: -r %t2.bc,v,x \ +; RUN: -r %t1.bc,foo,px \ +; RUN: -r %t2.bc,bar,px +; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s + +; Same as before, but reversing the order of the inputs +; RUN: llvm-lto2 %t2.bc %t1.bc -o %t.o -save-temps \ +; RUN: -r %t1.bc,v,px \ +; RUN: -r %t2.bc,v,x \ +; RUN: -r %t1.bc,foo,px \ +; RUN: -r %t2.bc,bar,px +; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s + + +; Client didn't mark any as prevailing, we keep the first one we see as "external" +; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \ +; RUN: -r %t1.bc,v,x \ +; RUN: -r %t2.bc,v,x \ +; RUN: -r %t1.bc,foo,px \ +; RUN: -r %t2.bc,bar,px +; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s + +; Same as before, but reversing the order of the inputs +; RUN: llvm-lto2 %t2.bc %t1.bc -o %t.o -save-temps \ +; RUN: -r %t1.bc,v,x \ +; RUN: -r %t2.bc,v,x \ +; RUN: -r %t1.bc,foo,px \ +; RUN: -r %t2.bc,bar,px +; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s + +target triple = "x86_64-apple-macosx10.11.0" + +@v = common global i8 0, align 8 + + +; CHECK: @v = common global [2 x i8] zeroinitializer, align 8 + +define i8 *@foo() { + ret i8 *@v +} \ No newline at end of file Index: llvm/trunk/tools/gold/gold-plugin.cpp =================================================================== --- llvm/trunk/tools/gold/gold-plugin.cpp +++ llvm/trunk/tools/gold/gold-plugin.cpp @@ -89,13 +89,6 @@ bool DefaultVisibility = true; }; -struct CommonResolution { - bool Prevailing = false; - bool VisibleToRegularObj = false; - uint64_t Size = 0; - unsigned Align = 0; -}; - } static ld_plugin_add_symbols add_symbols = nullptr; @@ -109,7 +102,6 @@ static std::list Modules; static DenseMap FDToLeaderHandle; static StringMap ResInfo; -static std::map Commons; static std::vector Cleanup; static llvm::TargetOptions TargetOpts; static size_t MaxTasks; @@ -572,12 +564,10 @@ toString(ObjOrErr.takeError()).c_str()); InputFile &Obj = **ObjOrErr; - bool HasThinLTOSummary = - hasGlobalValueSummary(Obj.getMemoryBufferRef(), diagnosticHandler); unsigned SymNum = 0; std::vector Resols(F.syms.size()); - for (auto &ObjSym : Obj.symbols()) { + for (LLVM_ATTRIBUTE_UNUSED auto &ObjSym : Obj.symbols()) { ld_plugin_symbol &Sym = F.syms[SymNum]; SymbolResolution &R = Resols[SymNum]; ++SymNum; @@ -619,21 +609,6 @@ (IsExecutable || !Res.DefaultVisibility)) R.FinalDefinitionInLinkageUnit = true; - if ((ObjSym.getFlags() & object::BasicSymbolRef::SF_Common) && - !HasThinLTOSummary) { - // We ignore gold's resolution for common symbols. A common symbol with - // the correct size and alignment is added to the module by the pre-opt - // module hook if any common symbol prevailed. - CommonResolution &CommonRes = Commons[ObjSym.getIRName()]; - if (R.Prevailing) { - CommonRes.Prevailing = true; - CommonRes.VisibleToRegularObj = R.VisibleToRegularObj; - } - CommonRes.Size = std::max(CommonRes.Size, ObjSym.getCommonSize()); - CommonRes.Align = std::max(CommonRes.Align, ObjSym.getCommonAlignment()); - R.Prevailing = false; - } - freeSymName(Sym); } @@ -668,32 +643,6 @@ } } -/// Add all required common symbols to M, which is expected to be the first -/// combined module. -static void addCommons(Module &M) { - for (auto &I : Commons) { - if (!I.second.Prevailing) - continue; - ArrayType *Ty = - ArrayType::get(Type::getInt8Ty(M.getContext()), I.second.Size); - GlobalVariable *OldGV = M.getNamedGlobal(I.first); - auto *GV = new GlobalVariable(M, Ty, false, GlobalValue::CommonLinkage, - ConstantAggregateZero::get(Ty), ""); - GV->setAlignment(I.second.Align); - if (OldGV) { - OldGV->replaceAllUsesWith(ConstantExpr::getBitCast(GV, OldGV->getType())); - GV->takeName(OldGV); - OldGV->eraseFromParent(); - } else { - GV->setName(I.first); - } - // We may only internalize commons if there is a single LTO task because - // other native object files may require the common. - if (MaxTasks == 1 && !I.second.VisibleToRegularObj) - GV->setLinkage(GlobalValue::InternalLinkage); - } -} - static CodeGenOpt::Level getCGOptLevel() { switch (options::OptLevel) { case 0: @@ -773,12 +722,6 @@ Conf.DiagHandler = diagnosticHandler; - Conf.PreOptModuleHook = [](size_t Task, Module &M) { - if (Task == 0) - addCommons(M); - return true; - }; - switch (options::TheOutputType) { case options::OT_NORMAL: break;