[PATCH] Use private linkage for the profile instrumentation variables that don't need to keep one copy when linking.

[Duncan P. N. Exon Smith]

> On 2014-Jul-24, at 13:13, Alex L <arphaman at gmail.com> wrote:
> 
>  4. For the "data" variables, I'm not convinced that changing `external`
>     variables to `private` is correct.  Consider how variables get
>     merged between translation units.  If you have a symbol that's
>     `weak` in one and `external` (strong) in another, references to the
>     weak symbol will point at the strong one instead.  If you change the
>     `global` one to `private`, the references to the weak symbol won't
>     get updated. This doesn't seem like the right behaviour.
> 
>     Since the "data" variables are in `llvm.used`, I think we'll end up
>     with two copies of them (one `weak`, the other `private`).  They'll
>     both point at counters, and it won't be clear which is canonical.
> 
> I haven't considered this point before, but this seems to be a real concern.
> Unfortunately this will mean that the size impact of private linkage will be very small.
> 
> The thing is that the `weak` and `external` symbols can represent functions with different
> source code, and I think for that case it makes sense not to merge the profile data for
> the `weak` function with the profile data for the `external` function. Therefore I believe
> that assigning `private` instead of `external` should work.
> 
> However, right now when the raw profile data ends up with two records for the function 
> with the same name, the profile data tool isn't able to handle the merging of them correctly
> as the hashes get mismatched because the functions have different source code. 
> Thus the profile data tool has to be updated to handle this case in order for this patch to be correct.

Hmm.  I'm not sure what the right answer is here.  Anyone else have an
opinion?

In the short therm, there's a somewhat boring blocker: the profile data
format can't store multiple functions that have the same name (unless
they have local linkage).  I believe counters from one of them will be
chosen arbitrarily.  In practice this comes up rarely (e.g., `main()`
from two different executables if their profiles get merged), but if we
stopped merging "data" variables it would be more common.

Even if `private` linkage even on "data" variables is the right
direction, I think this needs to be solved first.