[PGO] Don't group COMDAT variables for compiler generated profile variables in ELF

[xur]

Summary

With this patch, compiler generated profile variables will have its own COMDAT name for ELF format, which syncs the behavior with COFF.
Tested with clang PGO bootstrap. This shows a modest reduction in object sizes in ELF format.

[rnk]

lgtm, glad it saves object size.

This should remove the last call to getInstrProfComdatPrefix, so you can remove that too.

[MaskRay]

I think a potentially better approach is to reuse an existing symbol name as the comdat name (D84723). The state before this patch was inferior because __profv_ created a symbol which might waste .strtab space. By reusing an existing symbol (I picked __profd_), we don't waste .strtab space and can do better than this patch: we don't need to create one or two additional section groups (.group, sizeof(Elf64_Shdr)=64).

llvm/trunk/include/llvm/ProfileData/InstrProf.h

 inline StringRef getInstrProfCountersVarPrefix() { return "__profc_"; }
 
 /// Return the name prefix of value profile variables.
 inline StringRef getInstrProfValuesVarPrefix() { return "__profvp_"; }
 
 /// Return the name of value profile node array variables:
 inline StringRef getInstrProfVNodesVarName() { return "__llvm_prf_vnodes"; }
 
-/// Return the name prefix of the COMDAT group for instrumentation variables
-/// associated with a COMDAT function.
-inline StringRef getInstrProfComdatPrefix() { return "__profv_"; }
-
 /// Return the name of the variable holding the strings (possibly compressed)
 /// of all function's PGO names.
 inline StringRef getInstrProfNamesVarName() {
   return "__llvm_prf_nm";
 }
 
 /// Return the name of a covarage mapping variable (internal linkage)
 /// for each instrumented source module. Such variables are allocated

llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp

InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) {
   // Move the name variable to the right section. Place them in a COMDAT group
   // if the associated function is a COMDAT. This will make sure that only one
   // copy of counters of the COMDAT function will be emitted after linking. Keep
   // in mind that this pass may run before the inliner, so we need to create a
   // new comdat group for the counters and profiling data. If we use the comdat
   // of the parent function, that will result in relocations against discarded
   // sections.
   bool NeedComdat = needsComdatForCounter(*Fn, *M);
-  Comdat *Cmdt = nullptr; // Comdat group.
   if (NeedComdat) {
     if (TT.isOSBinFormatCOFF()) {
       // For COFF, put the counters, data, and values each into their own
       // comdats. We can't use a group because the Visual C++ linker will
       // report duplicate symbol errors if there are multiple external symbols
       // with the same name marked IMAGE_COMDAT_SELECT_ASSOCIATIVE.
       Linkage = GlobalValue::LinkOnceODRLinkage;
       Visibility = GlobalValue::HiddenVisibility;
-    } else {
-      // Otherwise, create one comdat group for everything.
-      Cmdt = M->getOrInsertComdat(getVarName(Inc, getInstrProfComdatPrefix()));
     }
   }
   auto MaybeSetComdat = [=](GlobalVariable *GV) {
     if (NeedComdat)
-      GV->setComdat(Cmdt ? Cmdt : M->getOrInsertComdat(GV->getName()));
+      GV->setComdat(M->getOrInsertComdat(GV->getName()));
   };
 
   uint64_t NumCounters = Inc->getNumCounters()->getZExtValue();
   LLVMContext &Ctx = M->getContext();
   ArrayType *CounterTy = ArrayType::get(Type::getInt64Ty(Ctx), NumCounters);
 
   // Create the counters variable.
   auto *CounterPtr =

llvm/trunk/test/Instrumentation/InstrProfiling/PR23499.ll

 ;; Check that PGO instrumented variables associated with linkonce odr
 ;; functions are placed in the same comdat section.
 
 
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -instrprof -S | FileCheck %s
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -passes=instrprof -S | FileCheck %s
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -instrprof -S | FileCheck %s
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=instrprof -S | FileCheck %s
 ; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -instrprof -S | FileCheck %s --check-prefix=COFF
 ; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -passes=instrprof -S | FileCheck %s --check-prefix=COFF
 
 $_Z3barIvEvv = comdat any
 
 @__profn__Z3barIvEvv = linkonce_odr hidden constant [11 x i8] c"_Z3barIvEvv", align 1
 
 ; CHECK-NOT: __profn__Z3barIvEvv
-; CHECK: @__profc__Z3barIvEvv = linkonce_odr hidden global [1 x i64] zeroinitializer, section "{{.*}}__llvm_prf_cnts", comdat($__profv__Z3barIvEvv), align 8
-; CHECK: @__profd__Z3barIvEvv = linkonce_odr hidden global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}__llvm_prf_data{{.*}}", comdat($__profv__Z3barIvEvv), align 8
+; CHECK: @__profc__Z3barIvEvv = linkonce_odr hidden global [1 x i64] zeroinitializer, section "{{.*}}__llvm_prf_cnts", comdat, align 8
+; CHECK: @__profd__Z3barIvEvv = linkonce_odr hidden global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}__llvm_prf_data{{.*}}", comdat, align 8
 ; CHECK: @__llvm_prf_nm = private constant [{{.*}} x i8] c"{{.*}}", section "{{.*}}__llvm_prf_names"
 
 
 ; COFF-NOT: __profn__Z3barIvEvv
 ; COFF: @__profc__Z3barIvEvv = linkonce_odr hidden global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat, align 8
 ; COFF: @__profd__Z3barIvEvv = linkonce_odr hidden global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat, align 8
 
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #1
 
 define linkonce_odr void @_Z3barIvEvv() comdat {
 entry:
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn__Z3barIvEvv, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
 }

llvm/trunk/test/Instrumentation/InstrProfiling/comdat.ll

 ; and COFF.
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
 
 $foo_inline = comdat any
 
 @__profn_foo_inline = linkonce_odr hidden constant [10 x i8] c"foo_inline"
 
-; ELF: @__profc_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_cnts", comdat($__profv_foo_inline), align 8
-; ELF: @__profd_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_data", comdat($__profv_foo_inline), align 8
+; ELF: @__profc_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_cnts", comdat, align 8
+; ELF: @__profd_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_data", comdat, align 8
 ; COFF: @__profc_foo_inline = linkonce_odr hidden global{{.*}}, section ".lprfc$M", comdat, align 8
 ; COFF: @__profd_foo_inline = linkonce_odr hidden global{{.*}}, section ".lprfd$M", comdat, align 8
 define weak_odr void @foo_inline() comdat {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_inline, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
 }
 
 $foo_extern = comdat any
 
 @__profn_foo_extern = linkonce_odr hidden constant [10 x i8] c"foo_extern"
 
-; ELF: @__profc_foo_extern = linkonce_odr hidden global{{.*}}, section "__llvm_prf_cnts", comdat($__profv_foo_extern)
-; ELF: @__profd_foo_extern = linkonce_odr hidden global{{.*}}, section "__llvm_prf_data", comdat($__profv_foo_extern)
+; ELF: @__profc_foo_extern = linkonce_odr hidden global{{.*}}, section "__llvm_prf_cnts", comdat, align 8
+; ELF: @__profd_foo_extern = linkonce_odr hidden global{{.*}}, section "__llvm_prf_data", comdat, align 8
 ; COFF: @__profc_foo_extern = linkonce_odr hidden global{{.*}}, section ".lprfc$M", comdat, align 8
 ; COFF: @__profd_foo_extern = linkonce_odr hidden global{{.*}}, section ".lprfd$M", comdat, align 8
 define available_externally void @foo_extern() {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_extern, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
 }

llvm/trunk/test/Instrumentation/InstrProfiling/linkage.ll

 ; POSIX: @__profd_foo_inline = linkonce_odr hidden global
 ; COFF: @__profc_foo_inline = internal global{{.*}} section ".lprfc$M", align 8
 ; COFF: @__profd_foo_inline = internal global{{.*}} section ".lprfd$M", align 8
 define linkonce_odr void @foo_inline() {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_inline, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
 }
 
-; LINUX: @__profc_foo_extern = linkonce_odr hidden global {{.*}}section "__llvm_prf_cnts", comdat($__profv_foo_extern), align 8
-; LINUX: @__profd_foo_extern = linkonce_odr hidden global {{.*}}section "__llvm_prf_data", comdat($__profv_foo_extern), align 8
+; LINUX: @__profc_foo_extern = linkonce_odr hidden global {{.*}}section "__llvm_prf_cnts", comdat, align 8
+; LINUX: @__profd_foo_extern = linkonce_odr hidden global {{.*}}section "__llvm_prf_data", comdat, align 8
 ; MACHO: @__profc_foo_extern = linkonce_odr hidden global
 ; MACHO: @__profd_foo_extern = linkonce_odr hidden global
 ; COFF: @__profc_foo_extern = linkonce_odr hidden global {{.*}}section ".lprfc$M", comdat, align 8
 ; COFF: @__profd_foo_extern = linkonce_odr hidden global {{.*}}section ".lprfd$M", comdat, align 8
 define available_externally void @foo_extern() {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_extern, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
 }

llvm/trunk/test/Transforms/PGOProfile/comdat_internal.ll

 ; RUN: opt < %s -pgo-instr-gen -instrprof -S | FileCheck %s
 ; RUN: opt < %s -passes=pgo-instr-gen,instrprof -S | FileCheck %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 $foo = comdat any
 ; CHECK: $foo = comdat any
 
 ; CHECK: $__llvm_profile_raw_version = comdat any
-; CHECK: $__profv__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any
 
 @bar = global i32 ()* @foo, align 8
 
 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
 ; CHECK-NOT: __profn__stdin__foo
-; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
+; CHECK: @__profc__stdin__foo.[[FOO_HASH:[0-9]+]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
 ; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null
 ; CHECK-NOT: bitcast (i32 ()* @foo to i8*)
-; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
+; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat, align 8
 ; CHECK: @__llvm_prf_nm
 ; CHECK: @llvm.used
 
 define internal i32 @foo() comdat {
 entry:
   ret i32 1
 }