[PATCH] D105176: [InstrProfiling] Use external weak reference for bias variable

[Petr Hosek via Phabricator via llvm-commits]

phosek added a comment.

In D105176#2865512 <https://reviews.llvm.org/D105176#2865512>, @MaskRay wrote:

> In D105176#2865477 <https://reviews.llvm.org/D105176#2865477>, @phosek wrote:
>
>> In D105176#2865370 <https://reviews.llvm.org/D105176#2865370>, @MaskRay wrote:
>>
>>>> We also disable the use of runtime counter relocation on Darwin since Mach-O doesn't support weak external references, but Darwin already uses a different continous mode that relies on overmapping so runtime counter relocation isn't needed there.
>>>
>>> I think you can restrict undefined weak to ELF now. PE-COFF doesn't have undefined weak, either.
>>
>> Done, do you know if there's any alternative we could use for PE-COFF?
>
> A `comdat any` is the easiest. The runtime defines a `comdat any` variable with value 0 which can be overridden by the program.

We need to distinguish the case where the variable is defined by the TU (in which case it has to be initialized with value 0) and where the variable is defined by the runtime.

Previously, the variable defined by the runtime would have the value -1 and the variable defined by the TU would have 0. After this change, runtime has an undefined reference and variable defined by TU is still initialized to 0.

Given that requirement, I'm not sure `comdat any` is going to work.

> It does rely on the link order but is the easiest.
>
> Alternatives include `IMAGE_SYM_CLASS_WEAK_EXTERNAL` (weak definition)  and linker directives `/alternative:` both of which are quite different/inconvenient.

That might work, I'll look into it.

> So I want to re-ask how the order difference is a problem. Using a `comdat any` can be straightforwardly ported to PE/COFF.

I noticed that in some of our instrumented binaries, the runtime definition initialized to -1 prevails in the binary. I haven't had pinpoint exactly why it's happening, in some cases we do mix C++ and Rust and manually link the profile runtime and I suspect that this might be one of the culprits.

I can dig into this a bit more and try to find, but I still think that relying on link order is fragile (especially since you won't notice the issue until you run the binary) and so I'd like to avoid it if possible.

>> I'll clarify the commit message, the issue is that relying on link order is fragile and can lead to hard-to-notice issues which we observed in practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105176