[PATCH] D56117: [ThinLTO] Scan all variants of vague symbol for reachability.

[Xin Tong via Phabricator via llvm-commits]

trentxintong marked an inline comment as done.
trentxintong added inline comments.


================
Comment at: test/ThinLTO/X86/deadstrip.ll:85
+
+; Make sure we keep @goo alive because one variant of @linkonceodrfuncwithalias
+; is calling it.
----------------
tejohnson wrote:
> Do we need goo to test the fix, or can't linkonceodrfuncwithalias be checked directly? The real issue seems to be that the prevailing copy of linkonceodrfuncwithalias in the second module is not kept without the fix. It should be kept and converted to weak_odr resolution, which you could check directly without having it call any functions.
Hi Teresa

The prevailing copy of linkonceodrfuncwithalias in the second module (Input/deadstrip.ll) is kept and converted to weak_odr resolution here. 

This fix is trying to address a bug when we do the mark-and-sweep to determine what is live (reachable). When we have an alias (@linkonceodralias in this case), it makes us mark the aliasee (@linkonceodrfuncwithalias in deadstrip.ll) alive and then walk the aliasee's references and calls which is nil here (https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/FunctionImport.cpp#L818). 

While this is OK, we have a problem if the same symbol is also reachable from somewhere else and some variants of it reference different set of global values, i.e. we would return early if one of the variants is alive without this patch (https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/FunctionImport.cpp#L780).

In the test case I have here, the symbol @linkonceodrfuncwithalias is first reached through the alias @linkonceodralias and second reached from @linkonceodrfuncwithalias_caller. I am testing we scan all variants of the symbol the second time its reached, instead of early return because its one instance of it is reached by the alias beforehand.



Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56117/new/

https://reviews.llvm.org/D56117