[PATCH] D32688: [Coverage] Comdat section name should be same as the variable name in COFF format

[Adam Folwarczny via llvm-commits]

Hello,

I have updated the alternative solution with internal linkage.
It looks like the previous solution (with naming) gives more similar
results to the Linux based version
(After short test I noticed that the same counters are merged during
linking on linux).

During bigger tests similar linker error I have also received with
___profn_ variable for classes with virtual destructors, but it looks like
completely different problem.
I should have more time to analyze this case in the next week.

Regards,
Adam

2017-05-03 23:40 GMT+02:00 Xinliang David Li <davidxl at google.com>:

> I am fine with a COFF specific change (either linkage or naming) assuming
> tests are fine.  Other platforms should be left unchanged.
>
> David
>
> On Wed, May 3, 2017 at 2:32 PM, Adam Folwarczny via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> adamf added a comment.
>>
>> > I suspect that the appropriate fix is to give the __profd variables
>> internal linkage, not to change the comdat name. The linker will still
>> discard duplicate profc and profd sections with this change.
>>
>> I have made few tests and changing to InternalLinkage seems also to work.
>> The patch is in attachment. F3280107: InstrProfiling.patch <
>> https://reviews.llvm.org/F3280107>.
>> This patch affects all formats, not only COFF.
>> With InternalLinkage we should change both _ _profd and _ _profc. (If we
>> change only _ _ profd then counters in the coverage report are not properly
>> interpreted).
>> Another advantage of using InternalLinkage is that we can check the
>> counters for each translation unit.
>>
>> Because fixing this problem starts to affect more amount of code and
>> requires wider knowledge I am afraid that I won't be able to provide fully
>> valuable fix.
>>
>>
>> https://reviews.llvm.org/D32688
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170510/03809b84/attachment.html>