[PATCH] D35702: [LTO][ThinLTO] Use the linker resolutions to mark global values as dso_local.
Sean Fertile via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 25 18:36:24 PDT 2017
sfertile marked 3 inline comments as done.
sfertile added inline comments.
================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:876
bool Live = (RawFlags & 0x2) || Version < 3;
- return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, Live);
+ bool Local = (RawFlags & 0x4);
+
----------------
mehdi_amini wrote:
> inouehrs wrote:
> > Do we need some version check (and some comments) for compatibility with old bc files?
> The default will be `0` for old .bc files, which translate to "not local", which should be conservatively OK I believe?
>
> Ideally we would checking a bitcode file from the 5.0 branching point in the repo and test this behavior.
That was my reasoning for not needing a version check.
I'll add the bitcode test from the 5.0 branch next update.
================
Comment at: lib/LTO/LTO.cpp:691
+ }
+ }
}
----------------
mehdi_amini wrote:
> This does not seem safe in face of hash collision.
I've limited the summary look-up to the bitcode module we are currently adding, is this what you had in mind?
================
Comment at: lib/LTO/LTO.cpp:685
+ auto GUID = GlobalValue::getGUID(
+ GlobalValue::dropLLVMManglingEscape(Sym.getIRName()));
+ if (auto S = ThinLTO.CombinedIndex.findSummaryInModule(
----------------
I realize that this is the same GUID as calculated above since the call to getGlobalIdentifier is being passed ExternalLinkage. I'll update this to reflect that.
Repository:
rL LLVM
https://reviews.llvm.org/D35702
More information about the llvm-commits
mailing list