[PATCH] Set function entry count.

[Justin Bogner]

Diego Novillo <dnovillo at google.com> writes:
> Hi dexonsmith,
>
> I don't particularly like this approach.
>
> The change in linkage for link once symbols is needed for ELF targets.
> Although the symbol is only associated once, the discarded symbols
> still occupy memory. This causes duplicate count problem at runtime
> because the raw profile contains multiple copies of the same data
> record (see https://llvm.org/bugs/show_bug.cgi?id=23499 for details).

I don't like it either. Changing everything to private wastes a ton of
space with duplicate counters.

If you need to do this for ELF targets to unblock you while you
investigate a better solution I'm okay with that, but:

1. It should be restricted to when we're targetting ELF as not to
   regress other targets.
2. We still need to figure out a better way.

In any case, PR23499 is a bug with or without the setEntryCount, so can
we split this into a patch that fixes that and another that sets the
count? Presumably we can test for PR23499 independently (though probably
only in the compiler-rt tests, since it depends on linker behaviour).

> http://reviews.llvm.org/D10035
>
> Files:
>   lib/CodeGen/CodeGenPGO.cpp
>   test/Profile/cxx-linkage.cpp
>   test/Profile/cxx-templates.cpp
>
> Index: lib/CodeGen/CodeGenPGO.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenPGO.cpp
> +++ lib/CodeGen/CodeGenPGO.cpp
> @@ -66,7 +66,8 @@
>    else if (Linkage == llvm::GlobalValue::AvailableExternallyLinkage)
>      Linkage = llvm::GlobalValue::LinkOnceODRLinkage;
>    else if (Linkage == llvm::GlobalValue::InternalLinkage ||
> -           Linkage == llvm::GlobalValue::ExternalLinkage)
> +           Linkage == llvm::GlobalValue::ExternalLinkage ||
> +           Linkage == llvm::GlobalValue::LinkOnceODRLinkage)
>      Linkage = llvm::GlobalValue::PrivateLinkage;
>  
>    auto *Value =
> @@ -773,6 +774,8 @@
>      // Turn on Cold attribute for cold functions.
>      // FIXME: 1% is from preliminary tuning on SPEC, it may not be optimal.
>      Fn->addFnAttr(llvm::Attribute::Cold);
> +
> +  Fn->setEntryCount(FunctionCount);
>  }
>  
>  void CodeGenPGO::emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S) {
> Index: test/Profile/cxx-linkage.cpp
> ===================================================================
> --- test/Profile/cxx-linkage.cpp
> +++ test/Profile/cxx-linkage.cpp
> @@ -3,7 +3,7 @@
>  // CHECK: @__llvm_profile_name__Z3foov = private constant [7 x i8] c"_Z3foov"
>  // CHECK: @__llvm_profile_name__Z8foo_weakv = weak hidden constant [12 x i8] c"_Z8foo_weakv"
>  // CHECK: @__llvm_profile_name_main = private constant [4 x i8] c"main"
> -// CHECK: @__llvm_profile_name__Z10foo_inlinev = linkonce_odr hidden constant [15 x i8] c"_Z10foo_inlinev"
> +// CHECK: @__llvm_profile_name__Z10foo_inlinev = private constant [15 x i8] c"_Z10foo_inlinev"
>  
>  void foo(void) { }
>  
> Index: test/Profile/cxx-templates.cpp
> ===================================================================
> --- test/Profile/cxx-templates.cpp
> +++ test/Profile/cxx-templates.cpp
> @@ -10,8 +10,8 @@
>  // RUN: FileCheck --input-file=%tuse -check-prefix=T0USE -check-prefix=ALL %s
>  // RUN: FileCheck --input-file=%tuse -check-prefix=T100USE -check-prefix=ALL %s
>  
> -// T0GEN: @[[T0C:__llvm_profile_counters__Z4loopILj0EEvv]] = linkonce_odr hidden global [2 x i64] zeroinitializer
> -// T100GEN: @[[T100C:__llvm_profile_counters__Z4loopILj100EEvv]] = linkonce_odr hidden global [2 x i64] zeroinitializer
> +// T0GEN: @[[T0C:__llvm_profile_counters__Z4loopILj0EEvv]] = private global [2 x i64] zeroinitializer
> +// T100GEN: @[[T100C:__llvm_profile_counters__Z4loopILj100EEvv]] = private global [2 x i64] zeroinitializer
>  
>  // T0GEN-LABEL: define linkonce_odr void @_Z4loopILj0EEvv()
>  // T0USE-LABEL: define linkonce_odr void @_Z4loopILj0EEvv()
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits