Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline
Changeset View
Changeset View
Standalone View
Standalone View
llvm/lib/LTO/LTO.cpp
| Show First 20 Lines • Show All 397 Lines • ▼ Show 20 Lines | |||||
| // Add the given symbol to the GlobalResolutions map, and resolve its partition. | // Add the given symbol to the GlobalResolutions map, and resolve its partition. | ||||
| void LTO::addSymbolToGlobalRes(const InputFile::Symbol &Sym, | void LTO::addSymbolToGlobalRes(const InputFile::Symbol &Sym, | ||||
| SymbolResolution Res, unsigned Partition) { | SymbolResolution Res, unsigned Partition) { | ||||
| auto &GlobalRes = GlobalResolutions[Sym.getName()]; | auto &GlobalRes = GlobalResolutions[Sym.getName()]; | ||||
| GlobalRes.UnnamedAddr &= Sym.isUnnamedAddr(); | GlobalRes.UnnamedAddr &= Sym.isUnnamedAddr(); | ||||
| if (Res.Prevailing) | if (Res.Prevailing) | ||||
| GlobalRes.IRName = Sym.getIRName(); | GlobalRes.IRName = Sym.getIRName(); | ||||
| // Set the partition to external if we know it is used elsewhere, e.g. | // Set the partition to external if we know it is re-defined by the linker | ||||
| // it is visible to a regular object, is referenced from llvm.compiler_used, | // with -defsym or -wrap options, used elsewhere, e.g. it is visible to a | ||||
| // or was already recorded as being referenced from a different partition. | // regular object, is referenced from llvm.compiler_used, or was already | ||||
| if (Res.VisibleToRegularObj || Sym.isUsed() || | // recorded as being referenced from a different partition. | ||||
| if (Res.LinkerRedefined || Res.VisibleToRegularObj || Sym.isUsed() || | |||||
davide: This comment is now stale after your change. | |||||
| (GlobalRes.Partition != GlobalResolution::Unknown && | (GlobalRes.Partition != GlobalResolution::Unknown && | ||||
| GlobalRes.Partition != Partition)) { | GlobalRes.Partition != Partition)) { | ||||
| GlobalRes.Partition = GlobalResolution::External; | GlobalRes.Partition = GlobalResolution::External; | ||||
| } else | } else | ||||
| // First recorded reference, save the current partition. | // First recorded reference, save the current partition. | ||||
| GlobalRes.Partition = Partition; | GlobalRes.Partition = Partition; | ||||
| // Flag as visible outside of ThinLTO if visible from a regular object or | // Flag as visible outside of ThinLTO if visible from a regular object or | ||||
| // if this is a reference in the regular LTO partition. | // if this is a reference in the regular LTO partition. | ||||
| GlobalRes.VisibleOutsideThinLTO |= | GlobalRes.VisibleOutsideThinLTO |= | ||||
| (Res.VisibleToRegularObj || Sym.isUsed() || | (Res.VisibleToRegularObj || Sym.isUsed() || | ||||
If I remove this line, all the tests still pass. davide: If I remove this line, all the tests still pass. | |||||
Not Done ReplyInline ActionsThis was meant for ThinLTO support, which hasn't been implemented yet. I can take it out from this patch. dmikulin: This was meant for ThinLTO support, which hasn't been implemented yet. I can take it out from… | |||||
| Partition == GlobalResolution::RegularLTO); | Partition == GlobalResolution::RegularLTO); | ||||
| } | } | ||||
Not Done ReplyInline Actionswhy do you need this being part of GlobalResolution? davide: why do you need this being part of GlobalResolution?
If you do, please add a test showing that… | |||||
Not Done ReplyInline ActionsYou're right, I can do it sooner, in addRegularLTO(). I'll take out the GlobalRes stuff. dmikulin: You're right, I can do it sooner, in addRegularLTO(). I'll take out the GlobalRes stuff. | |||||
| static void writeToResolutionFile(raw_ostream &OS, InputFile *Input, | static void writeToResolutionFile(raw_ostream &OS, InputFile *Input, | ||||
| ArrayRef<SymbolResolution> Res) { | ArrayRef<SymbolResolution> Res) { | ||||
| StringRef Path = Input->getName(); | StringRef Path = Input->getName(); | ||||
| OS << Path << '\n'; | OS << Path << '\n'; | ||||
| auto ResI = Res.begin(); | auto ResI = Res.begin(); | ||||
| for (const InputFile::Symbol &Sym : Input->symbols()) { | for (const InputFile::Symbol &Sym : Input->symbols()) { | ||||
| assert(ResI != Res.end()); | assert(ResI != Res.end()); | ||||
| SymbolResolution Res = *ResI++; | SymbolResolution Res = *ResI++; | ||||
| OS << "-r=" << Path << ',' << Sym.getName() << ','; | OS << "-r=" << Path << ',' << Sym.getName() << ','; | ||||
| if (Res.Prevailing) | if (Res.Prevailing) | ||||
| OS << 'p'; | OS << 'p'; | ||||
| if (Res.FinalDefinitionInLinkageUnit) | if (Res.FinalDefinitionInLinkageUnit) | ||||
| OS << 'l'; | OS << 'l'; | ||||
| if (Res.VisibleToRegularObj) | if (Res.VisibleToRegularObj) | ||||
| OS << 'x'; | OS << 'x'; | ||||
| if (Res.LinkerRedefined) | |||||
| OS << 'r'; | |||||
If I comment out this line, all the tests still pass. davide: If I comment out this line, all the tests still pass. | |||||
Not Done ReplyInline ActionsAdded a test point for this dmikulin: Added a test point for this | |||||
You also need to add support for 'r' to llvm-lto2. Please also add an llvm-lto2 based test that verifies that the linkage has changed. pcc: You also need to add support for `'r'` to llvm-lto2. Please also add an llvm-lto2 based test… | |||||
| OS << '\n'; | OS << '\n'; | ||||
| } | } | ||||
| OS.flush(); | OS.flush(); | ||||
| assert(ResI == Res.end()); | assert(ResI == Res.end()); | ||||
| } | } | ||||
| Error LTO::add(std::unique_ptr<InputFile> Input, | Error LTO::add(std::unique_ptr<InputFile> Input, | ||||
| ArrayRef<SymbolResolution> Res) { | ArrayRef<SymbolResolution> Res) { | ||||
| ▲ Show 20 Lines • Show All 88 Lines • ▼ Show 20 Lines | for (const InputFile::Symbol &Sym : Syms) { | ||||
| ModuleSymbolTable::Symbol Msym = *MsymI++; | ModuleSymbolTable::Symbol Msym = *MsymI++; | ||||
| Skip(); | Skip(); | ||||
| if (GlobalValue *GV = Msym.dyn_cast<GlobalValue *>()) { | if (GlobalValue *GV = Msym.dyn_cast<GlobalValue *>()) { | ||||
| if (Res.Prevailing) { | if (Res.Prevailing) { | ||||
| if (Sym.isUndefined()) | if (Sym.isUndefined()) | ||||
| continue; | continue; | ||||
| Keep.push_back(GV); | Keep.push_back(GV); | ||||
| // For symbols re-defined with linker -wrap and -defsym options, | |||||
| // set the linkage to weak to inhibit IPO. The linkage will be | |||||
This needs a comment. davide: This needs a comment. | |||||
| // restored by the linker. | |||||
| if (Res.LinkerRedefined) | |||||
| GV->setLinkage(GlobalValue::WeakAnyLinkage); | |||||
| switch (GV->getLinkage()) { | switch (GV->getLinkage()) { | ||||
| default: | default: | ||||
| break; | break; | ||||
| case GlobalValue::LinkOnceAnyLinkage: | case GlobalValue::LinkOnceAnyLinkage: | ||||
| GV->setLinkage(GlobalValue::WeakAnyLinkage); | GV->setLinkage(GlobalValue::WeakAnyLinkage); | ||||
| break; | break; | ||||
| case GlobalValue::LinkOnceODRLinkage: | case GlobalValue::LinkOnceODRLinkage: | ||||
| GV->setLinkage(GlobalValue::WeakODRLinkage); | GV->setLinkage(GlobalValue::WeakODRLinkage); | ||||
| ▲ Show 20 Lines • Show All 517 Lines • Show Last 20 Lines | |||||
This comment is now stale after your change.