[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-23, at 10:57, Alex L <arphaman at gmail.com> wrote:
> 
> Hi,
> 
> I've attached a patch that forces the per-function profile instrumentation variables to use the private linkage unless they need to use the linkage that keeps only one copy. 
> 
> The private linkage forces the names for those variables to be stripped, reducing the resulting object file size.
> 
> Alex
> <PGOprivateLinkage.patch>

Thanks Alex, this sounds promising.

 1. Can you post numbers for the file size reduction?

 2. I think it makes sense to change any `internal` symbols to
    `private`.

 3. I suspect you could change *all* "name" variables to `private`.
    Since they're already dumped in a special section, I suspect you
    could go further and mark them with `unnamed_addr`, allowing them to
    be merged with each other.

 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 might be missing something.  Can you check if there's a testcase
    for that case in the compiler-rt tests (`check-profile`) and the
    clang tests (`test/Profile`)?  If not, that was an oversight --
    please add tests for both test suites (and make sure they pass with
    your change).

 5. For the "counter" variables, your logic is probably fine.  What we
    need to be sure of is that the surviving definition of the function
    in question uses the *same* counters that the surviving "data"
    variable points to.

 6. I'm surprised no clang tests failed after this change (in
    particular, I'd have expected `test/profile/*-linkage.*` to start
    failing).  If they are failing, can you update them?  If they
    aren't, can you add test coverage for this?