[PATCH] D41988: [ThinLTO] - Set WeakAnyLinkage for all LinkerRedefined symbols.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 15 08:15:03 PST 2018


>> Ok I see what happens. In addRegularLTO only prevailing symbols are added
>> to the Keep set used when linking the IR (and a few other cases like
>> linkonce_odr). In ThinLTO, obviously we don't have the IR during the thin
>> link.
>>
>> Here's what we currently do with the prevailing info for ThinLTO:
>> - in addThinLTO we already keep track of the prevailing module for a symbol
>> (PrevailingModuleForGUID)
>> - this is only used right now in weak symbol resolution - for linkonce, we
>> upgrade the prevailing symbol to weak, and for other WeakForLinker symbols
>> we downgrade the non-prevailing to available_externally (when legal)
>>
>> What we probably can/should do: Use PrevailingModuleForGUID to:
>> 1) prevent importing of the non-prevailing copy when invoking
>> ComputeCrossModuleImport
>> 2) force the non-prevailing symbols to be internalized
>> via thinLTOInternalizeAndPromoteInIndex
>>
>> For #2, we can assert in isExported that anything in an ExportList (because
>> it was imported) is the prevailing copy (after doing #1). Otherwise, if the
>> symbol is not prevailing in that module then return false from isExported
>> to ensure it is internalized.
>>
>> Does that sound right?
>
>Part 1 sounds right. For part 2 I don't think that internalizing is
>correct, we want to drop the symbol completely.
>
>Thanks a lot for the description, it was a bit hard to get the big
>picture by reading the code.
>
>From the description I was able to create a testcase that shows why
>internalizing symbols is not sufficient, we have to drop them:
>
>https://bugs.llvm.org/show_bug.cgi?id=35938
>

Thanks ! Reproduced issue with testcase provided and 
going to start debugging of it tomorrow (if nobody will take and/or fix it earlier).

George.


More information about the llvm-commits mailing list