[PATCH] D38239: [ELF] - Define linkerscript symbols early.

[Rafael Avila de Espindola via llvm-commits]

George Rimar <grimar at accesssoftek.com> writes:


> Part II. "ThinLTO fix for defsym.ll"
> (patch for below is in attachment)
>
> Currently LTO sets WeakAnyLinkage for all symbols that are Prevailing and have LinkerRedefined flag set.
> I am not sure I understand why it is not done just for all LinkerRedefined symbols. 
> According to comment there "we want to switch the linkage to `weak` to prevent IPOs from happening",
> so dont we need to do that for all linker redefined symbols, like symbols used in -defsym ?
> Changing that here:
> (https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/LTO.cpp#L717)
>
> stops `bar2` from being exported:
> https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/FunctionImport.cpp#L181
>
> and then LTO sets `bar2` linkage to internal here:
> https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/LTO.cpp#L329
>
> So `bar2` never shows up and we have no multiple defenition error anymore.

This seems to be a bit of a hack. With it llvm sets the linkage to weak
to prevent inlining and that ends up avoiding the symbol for being used,
but that seems to happen in a profitability logic, not correctness.

I have attached a combined patch for llvm+lld. It does pass every test,
but we have to figure out the real fix for thin lto. It should not be
generating a symbol that is not prevailing.

I would suggest starting a new review with the attached patch and adding
Peter and Teresa as reviewers as we could really use some help from a
ThinLTO expert.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.diff
Type: text/x-patch
Size: 12130 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180111/fe481b87/attachment.bin>
-------------- next part --------------

Thanks,
Rafael