[InstrProfiling] Use llvm.compiler.used instead of llvm.used for ELF

[MaskRay]

Summary

Many optimizers (e.g. GlobalOpt/ConstantMerge) do not respect linker semantics
for comdat and may not discard the sections as a unit.

The interconnected __llvm_prf_{cnts,data} sections (in comdat for ELF)
are similar to D97432: __profd_ is not directly referenced, so
__profd_ may be discarded while __profc_ is retained, breaking the
interconnection. We currently conservatively add all such sections to
llvm.used and let the linker do GC for ELF.

In D97448, we will change GlobalObject's in the llvm.used list to use SHF_GNU_RETAIN,
causing the metadata sections to be unnecessarily retained (some check-profile tests check for GC).
Use llvm.compiler.used to retain the current GC behavior.

[davidxl]

So to summarize, this patch is to enable compiler passes to respect comdat semantics while still enabling linker to do GC (after using GNU_RETAIN)?

[MaskRay]

In D97585#2591568, @davidxl wrote:

So to summarize, this patch is to enable compiler passes to respect comdat semantics while still enabling linker to do GC (after using GNU_RETAIN)?

The patch makes the ELF behavior the same (the metadata sections can be GCed) before or after D97448.

The comments and the paragraph in this patch are mainly to summarize the current situation (why we use llvm.compiler.used/llvm.used) :)

[davidxl]

lgtm for ELF. Anything to do for COFF?

[MaskRay]

In D97585#2591594, @davidxl wrote:

lgtm for ELF. Anything to do for COFF?

I think the existing behavior may be a bit problematic for COFF, so let @rnk know and add some tests demonstrating the behavior:)

Perhaps COFF should switch to comdat and llvm.compiler.used as well. Without comdat, because currently llvm.used does not retain sections defining PrivateLinkage/InternalLinkage symbols, the GC result may be weird.

[rnk]

Wait, are you telling me that __profd/__profc data can participate in linker GC, and that is not a problem? I have actually had to optimize the processing of /INCLUDE: directives for COFF (D78845), because I assumed that it was critical that __profd be treated as a GC root. But, on ELF, they were never GC roots, and coverage/PGO has worked just fine there. So, really, all along, llvm.used was only being used to block LLVM IPO transforms like globalopt, not to establish a GC root. And, as you point out, we don't get the GC root behavior for internal __profd/__profc globals, and coverage/PGO seems to work fine.

What about MachO? Should all platforms not treat __profd as a GC root?

[rnk]

Switching to compiler.used will save ~8% of PGO object file size, although these numbers are stale: https://crbug.com/1058040#c14 I filed an issue to prototype an alternative GC root object file format similar to .llvm_addrsig in an effort to capture those object file savings, and you are telling me that none of that was ever necessary!

[MaskRay]

In D97585#2595131, @rnk wrote:

Wait, are you telling me that __profd/__profc data can participate in linker GC, and that is not a problem?

Yes on ld.lld, since D96636 . The GC is more useful with D96757, which moved metadata sections which were not within a comdat any into a comdat noduplicates group.

I have actually had to optimize the processing of /INCLUDE: directives for COFF (D78845), because I assumed that it was critical that __profd be treated as a GC root.

Yes. I think getting rid of the local linkage limitation is important for COFF GC.

But, on ELF, they were never GC roots, and coverage/PGO has worked just fine there.

Yes. For some sections (__llvm_prf_nm __llvm_prf_vnds), they work just because linkers have such a rule:

__start_/__stop_ references from a live input section retains all non-SHF_LINK_ORDER non-SHF_GROUP C identifier name sections.

-z start-stop-gc can drop the rule, so we need D97649 for robustness.

That said, I am still not very confident with GCness for the compressed __llvm_prf_nm. I think for PGO the section is optional, so whether we retain or discard it probably does not matter much. For coverage mapping it might be a problem.
But this patch and D97649 do not change the status quo.

So, really, all along, llvm.used was only being used to block LLVM IPO transforms like globalopt, not to establish a GC root. And, as you point out, we don't get the GC root behavior for internal __profd/__profc globals, and coverage/PGO seems to work fine.

What about MachO? Should all platforms not treat __profd as a GC root?

Since MachO does not have comdat. I think it has to be conservative and treats every metadata section not directly or indirectly referenced by code as GC roots (llvm.used).

All binary formats with ELF section group compatible semantics ("if section A in a section group is retained, all members in the group are retained as well") should be able to use llvm.compiler.used.
I believe COFF can do it, but as you said, /INCLUDE: needs fixes to support local linkage GlobalObjects.

[phosek]

Correct me if I'm wrong, but I assumed we could use the same approach that
was suggested in https://bugs.llvm.org/show_bug.cgi?id=49155 for
__llvm_prf_nm.

That is, instead of emitting a single per-TU symbol with all the names, we
would emit one symbol for each name, all having the same section name. When
lowering, all those symbols get combined into a SHF_MERGE & SHF_STRINGS
section. We could then put each symbol into a group with the other
per-function symbols.

I tried this with coverage mapping locally and it's working well. The
problem is with COFF and Mach-O since they AFAIK don't have an equivalent
of SHF_COMPRESSED so we would lose the benefit of compression there.

We could continue using the current scheme for COFF and Mach-O and only use
the proposed approach (that is string merging with compression handled
transparently by the backend & the linker) for ELF, but it'd mean more
divergence between the formats which may be undesirable.

• smime.p7s3 KBDownload

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

bool InstrProfiling::emitRuntimeHook() {
   IRB.CreateRet(Load);
 
   // Mark the user variable as used so that it isn't stripped out.
   UsedVars.push_back(User);
   return true;
 }
 
 void InstrProfiling::emitUses() {
-  if (!UsedVars.empty())
+  // The metadata sections are parallel arrays. Optimizers (e.g.
+  // GlobalOpt/ConstantMerge) may not discard associated sections as a unit, so
+  // we conservatively retain all unconditionally in the compiler.
+  //
+  // On ELF, the linker can guarantee the associated sections will be retained
+  // or discarded as a unit, so llvm.compiler.used is sufficient. Otherwise,
+  // conservatively make all of them retained by the linker.
+  if (TT.isOSBinFormatELF())
+    appendToCompilerUsed(*M, UsedVars);
+  else
     appendToUsed(*M, UsedVars);
 }
 
 void InstrProfiling::emitInitialization() {
   // Create ProfileFileName variable. Don't don't this for the
   // context-sensitive instrumentation lowering: This lowering is after
   // LTO/ThinLTO linking. Pass PGOInstrumentationGenCreateVar should
   // have already create the variable before LTO/ThinLTO linking.

llvm/test/Instrumentation/InstrProfiling/profiling.ll

-; RUN: opt < %s -instrprof -S | FileCheck %s
-; RUN: opt < %s -passes=instrprof -S | FileCheck %s
+; RUN: opt < %s -mtriple=x86_64 -passes=instrprof -S | FileCheck %s --check-prefixes=CHECK,ELF
+; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -passes=instrprof -S | FileCheck %s --check-prefixes=CHECK,MACHO
+; RUN: opt < %s -mtriple=x86_64-windows -passes=instrprof -S | FileCheck %s --check-prefixes=CHECK,WIN
 
-target triple = "x86_64-apple-macosx10.10.0"
+; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -instrprof -S | FileCheck %s
 
 ; CHECK: @__llvm_profile_runtime = external global i32
 
 @__profn_foo = hidden constant [3 x i8] c"foo"
 ; CHECK-NOT: __profn_foo
 @__profn_bar = hidden constant [4 x i8] c"bar\00"
 ; CHECK-NOT: __profn_bar
 @__profn_baz = hidden constant [3 x i8] c"baz"
 ; CHECK-NOT: __profn_baz
 
-; CHECK: @__profc_foo = hidden global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
-; CHECK: @__profd_foo = hidden {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
+; ELF:   @__profc_foo = hidden global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_foo), align 8
+; ELF:   @__profd_foo = hidden {{.*}}, section "__llvm_prf_data", comdat, align 8
+; MACHO: @__profc_foo = hidden global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
+; MACHO: @__profd_foo = hidden {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
+; WIN:   @__profc_foo = internal global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
+; WIN:   @__profd_foo = internal {{.*}}, section ".lprfd$M", align 8
 define void @foo() {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
 }
 
-; CHECK: @__profc_bar = hidden global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
-; CHECK: @__profd_bar = hidden {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
+; ELF:   @__profc_bar = hidden global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_bar), align 8
+; ELF:   @__profd_bar = hidden {{.*}}, section "__llvm_prf_data", comdat, align 8
+; MACHO: @__profc_bar = hidden global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
+; MACHO: @__profd_bar = hidden {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
+; WIN:   @__profc_bar = internal global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
+; WIN:   @__profd_bar = internal {{.*}}, section ".lprfd$M", align 8
 define void @bar() {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @__profn_bar, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
 }
 
-; CHECK: @__profc_baz = hidden global [3 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
-; CHECK: @__profd_baz = hidden {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
+; ELF:   @__profc_baz = hidden global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_baz), align 8
+; ELF:   @__profd_baz = hidden {{.*}}, section "__llvm_prf_data", comdat, align 8
+; MACHO: @__profc_baz = hidden global [3 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
+; MACHO: @__profd_baz = hidden {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
+; WIN:   @__profc_baz = internal global [3 x i64] zeroinitializer, section ".lprfc$M", align 8
+; WIN:   @__profd_baz = internal {{.*}}, section ".lprfd$M", align 8
 define void @baz() {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_baz, i32 0, i32 0), i64 0, i32 3, i32 0)
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_baz, i32 0, i32 0), i64 0, i32 3, i32 1)
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_baz, i32 0, i32 0), i64 0, i32 3, i32 2)
   ret void
 }
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
 
-; CHECK: @llvm.used = appending global {{.*}} @__profd_foo {{.*}} @__profd_bar {{.*}} @__profd_baz {{.*}} section "llvm.metadata"
+; ELF:   @llvm.compiler.used = appending global {{.*}} @__llvm_profile_runtime_user {{.*}} @__profd_foo {{.*}} @__profd_bar {{.*}} @__profd_baz
+; MACHO: @llvm.used = appending global {{.*}} @__llvm_profile_runtime_user {{.*}} @__profd_foo {{.*}} @__profd_bar {{.*}} @__profd_baz
+; WIN:   @llvm.used = appending global {{.*}} @__llvm_profile_runtime_user {{.*}} @__profd_foo {{.*}} @__profd_bar {{.*}} @__profd_baz