[PATCH] D26076: [ThinLTO] Correctly resolve linkonce when importing aliasee

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.


================
Comment at: lib/LTO/LTO.cpp:123
 
 static void thinLTOResolveWeakForLinkerGUID(
     GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID,
----------------
tejohnson wrote:
> mehdi_amini wrote:
> > It is not clear to me right now why is it part of this stage. Conceptually it seems that you are solving a "promotion" issue, and not a "weak resolution" thing.
> The code below here converts the prevailing linkonce to weak to ensure one copy is kept - even if there are no other copies that will be converted to available_externally. This is not new and ensures if we export a reference to that linkonce that it will be satisfied at link time (so that particular part of this routine is in fact like a promotion).
> 
> The change in this patch is to allow that part to occur (the correctness part) for an aliasee. It was a missing case, and was resulting in some undefs at link time in certain cases (e.g. the unsats davidxl got when trying to link Clang with ThinLTO + PGO last week).
> 
> We could either just better document this part of the handling in this routine, or split it out - e.g a separate pass of promoting linkonce->weak when the linkonce is exported, but that seems orthogonal to fixing the existing handling for this specific case (and somewhat redundant with the linkonce->weak that is done here already).
Note also this routine used to explicitly check for and handle the exported case, but it was removed with the idea that we would always do the linkonce->weak promotion for the prevailing copy (see r274722 and r274784).


https://reviews.llvm.org/D26076