[PATCH] D20268: [wip] Resolution-based LTO API.

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 14 08:27:46 PDT 2016


tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM, with one nit below.

However, I'd like to wait to commit this until:

1. Mehdi signs off
2. https://reviews.llvm.org/D22302 goes in, enabling weak/linkonce resolution for gold. Your patch has the side effect of doing this enabling, and I think it is cleaner to have that functionality added separately from this patch adding the new API.

Mehdi, can you take a look at this patch and https://reviews.llvm.org/D22302 asap - this is getting on the critical path of some fixes and follow on patches I need to add for the distributed backend case (will send a separate update on that).


================
Comment at: lib/LTO/Resolution/LTOBackend.cpp:32
@@ +31,3 @@
+    Hook = [=](size_t Task, Module &M) {
+      if (OldHook && !OldHook(Task, M))
+        return false;
----------------
pcc wrote:
> tejohnson wrote:
> > I'm confused by what is going on here with OldHook.
> This is calling the hook provided by the linker, which still needs to run (e.g. in order to add common symbols). If the linker's hook returned false, we need to pass that result through.
Ok, maybe rename to LinkerHook and/or add a comment.


https://reviews.llvm.org/D20268





More information about the llvm-commits mailing list