[llvm] r238335 - Fix PR 23499 - Avoid multiple profile counters for functions in comdat sections.

[Xinliang David Li]

Why is Name not 'comdatted' ?

Multiple copies of counters for comdat functions might actually be a
feature instead of a bug. Consider the case when instrumented comdat
function gets inlined into the caller.  Keeping different copies of
counters allows more context sensitive profiling while de-duping the
counters causes the profiles of inline instances to be merged into the out
of line instance.

This is a moot point for now as there is no profile-use mechanism to take
advantage of that context-sensitivity (i.e., using inline instance profile
copy during inline profile update). So this patch is fine for now and can
be revisited later if we lose perf opportunities).

thanks,

David


On Wed, May 27, 2015 at 9:44 AM, Diego Novillo <dnovillo at google.com> wrote:

> Author: dnovillo
> Date: Wed May 27 11:44:47 2015
> New Revision: 238335
>
> URL: http://llvm.org/viewvc/llvm-project?rev=238335&view=rev
> Log:
> Fix PR 23499 - Avoid multiple profile counters for functions in comdat
> sections.
>
> Counter symbols created for linkonce functions are not discarded by ELF
> linkers unless the symbols are placed in the same comdat section as its
> associated function.
>
> Modified:
>     llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp
>
> Modified: llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp?rev=238335&r1=238334&r2=238335&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp (original)
> +++ llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp Wed May
> 27 11:44:47 2015
> @@ -203,6 +203,7 @@ InstrProfiling::getOrCreateRegionCounter
>    uint64_t NumCounters = Inc->getNumCounters()->getZExtValue();
>    LLVMContext &Ctx = M->getContext();
>    ArrayType *CounterTy = ArrayType::get(Type::getInt64Ty(Ctx),
> NumCounters);
> +  Function *Fn = Inc->getParent()->getParent();
>
>    // Create the counters variable.
>    auto *Counters = new GlobalVariable(*M, CounterTy, false,
> Name->getLinkage(),
> @@ -211,6 +212,10 @@ InstrProfiling::getOrCreateRegionCounter
>    Counters->setVisibility(Name->getVisibility());
>    Counters->setSection(getCountersSection());
>    Counters->setAlignment(8);
> +  // Place the counters in the same comdat section as its parent function.
> +  // Otherwise, we may get multiple counters for the same function in
> certain
> +  // cases.
> +  Counters->setComdat(Fn->getComdat());
>
>    RegionCounters[Inc->getName()] = Counters;
>
> @@ -235,6 +240,7 @@ InstrProfiling::getOrCreateRegionCounter
>    Data->setVisibility(Name->getVisibility());
>    Data->setSection(getDataSection());
>    Data->setAlignment(8);
> +  Data->setComdat(Fn->getComdat());
>
>    // Mark the data variable as used so that it isn't stripped out.
>    UsedVars.push_back(Data);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150527/665b01ee/attachment.html>