[PATCH] D37059: [ELF] - LTO: do not optimize away symbols accessed from linkerscript.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/LTO.cpp:170
       undefine(Sym);
     R.LinkerRedefined = Config->RenamedSymbols.count(Sym);
   }
----------------
grimar wrote:
> pcc wrote:
> > I think we should be setting `LinkerRedefined` to true for these symbols instead of `VisibleToRegularObj`, otherwise the optimizer may IPO past them.
> I am not really familar with LTO logic, but if that is true, that sounds like a bug of LTO for me. 
> May be I am missing something, but if linker says symbols should be visible (and that means used) from regular object,
> I would expect LTO should not eliminate them out.
> 
> But actually I think it does not do that:
> I looked how both `VisibleToRegularObj` and `LinkerRedefined` are used. They set symbol resolution to
> GlobalResolution::External:
> https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/LTO.cpp#L426
> 
> And that prevents making them internal here:
> https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/LTO.cpp#L812
> 
> After all when parsing LTO objects and symbols:
> https://github.com/llvm-mirror/lld/blob/master/ELF/SymbolTable.cpp#L125
> there is no `bar` symbol at all (because `VisibleToRegularObj` was not set for it,
> but `foo` is still there). That probably shows that logic is correct.
> What am I missing ?
> 
When I mentioned `bar` and `foo`, I meant those from my testcase. `foo` is assigned in script, `bar` is not.


https://reviews.llvm.org/D37059