Fix PR 23499

[Justin Bogner]

Diego Novillo <dnovillo at google.com> writes:
> Based on our IRC discussion, I think this will address the multiple
> copy problem on ELF systems.  By placing the counters in the same
> comdat section as its associated function, we get the linker to
> discard them together with the discarded function.

Is setting the comdat on Data sufficient, or should we do it for
Counters (and possibly even name) also? I guess that could lead to some
more space savings, even if it doesn't have a visible effect.

> I don't think we need to create another test case just to trigger this
> bug.  A small modification to the existing test case exposes it. I can
> create a new one, if you'd rather have a different test case for this,
> though.
>
> OK to commit?

I agree about the test. This LGTM once you've looked into the comment
above.

>
> Thanks.  Diego.
>
> diff --git a/lib/Transforms/Instrumentation/InstrProfiling.cpp b/lib/Transforms/Instrumentation/InstrProfiling.cpp
> index 0718ddc..b12c475 100644
> --- a/lib/Transforms/Instrumentation/InstrProfiling.cpp
> +++ b/lib/Transforms/Instrumentation/InstrProfiling.cpp
> @@ -235,6 +235,12 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) {
>    Data->setVisibility(Name->getVisibility());
>    Data->setSection(getDataSection());
>    Data->setAlignment(8);
> +  // Place the data variable in the same comdat section as
> +  // its parent function. Otherwise, we may get multiple
> +  // counters for the same function in certain cases
> +  // (https://llvm.org/bugs/show_bug.cgi?id=23499).

We usually wrap comments a little wider than this, and there's no need
to refer to the bug number here.

> +  Function* F = Inc->getParent()->getParent();
> +  Data->setComdat(F->getComdat());
>  
>    // Mark the data variable as used so that it isn't stripped out.
>    UsedVars.push_back(Data);
> diff --git a/test/profile/Inputs/instrprof-dynamic-header.h b/test/profile/Inputs/instrprof-dynamic-header.h
> index 1dc2e37..4d4a513 100644
> --- a/test/profile/Inputs/instrprof-dynamic-header.h
> +++ b/test/profile/Inputs/instrprof-dynamic-header.h
> @@ -1,5 +1,8 @@
> +extern int X;
>  template <class T> void bar() {
> -  if (true) {}
> +  if (X) {
> +    X *= 4;
> +  }
>  }
>  void a();
>  void b();
> diff --git a/test/profile/Inputs/instrprof-dynamic-main.cpp b/test/profile/Inputs/instrprof-dynamic-main.cpp
> index 0dd6021..5ba46ad 100644
> --- a/test/profile/Inputs/instrprof-dynamic-main.cpp
> +++ b/test/profile/Inputs/instrprof-dynamic-main.cpp
> @@ -1,7 +1,9 @@
>  #include "instrprof-dynamic-header.h"
> +int X = 0;
>  void foo(int K) { if (K) {} }
>  int main(int argc, char *argv[]) {
>    foo(5);
> +  X++;
>    bar<void>();
>    a();
>    b();