[InstrProfiling] Use nobits as __llvm_prf_cnts section type in ELF

[phosek]

Summary

This can reduce the binary size because counters will no longer occupy
space in the binary, instead they will be allocated by dynamic linker.

[davidxl]

lgtm

[aeubanks]

I'm looking into https://crbug.com/1180779 where a bunch of profile tests are failing with

Malformed instrumentation profile data
 error: No profiles could be merged.

This is the only commit that looks related in the regression range.

I also found https://lab.llvm.org/buildbot/#/builders/93/builds/1897 which has similar failures.

Is it possible for this change to cause these failures? Perhaps the host has an old static/dynamic linker that doesn't support something this requires?

[Conanap]

Hello,

Power PC is also experiencing the same error,

Malformed instrumentation profile data
 error: No profiles could be merged.

I built a branch of main with this change reverted and the error goes away. Would it be possible to have this reverted for now until a proper solution has been created? Thanks!

[phosek]

In D97110#2583026, @Conanap wrote:

Hello,

Power PC is also experiencing the same error,

Malformed instrumentation profile data
 error: No profiles could be merged.

I built a branch of main with this change reverted and the error goes away. Would it be possible to have this reverted for now until a proper solution has been created? Thanks!

I've managed to reproduce this locally in a Docker container. It's a bug in the older binutils bfd ld, the __start_ and __stop_ have incorrect addresses, it's working correctly in gold and also in newer binutils versions. I'll send a change to conditionally enable this feature with -fbinutils-version=, I'm just trying to identify what version of binutils this was fixed in.

[davidxl]

Kazu has seen this error

error: SHT_NOBITS section '__llvm_prf_cnts' cannot have non-zero
initializers

This is with lld.

David

[phosek]

Do you know what's their setup or how to reproduce it?

[phosek]

In D97110#2583082, @phosek wrote:

In D97110#2583026, @Conanap wrote:

Hello,

Power PC is also experiencing the same error,

Malformed instrumentation profile data
 error: No profiles could be merged.

I built a branch of main with this change reverted and the error goes away. Would it be possible to have this reverted for now until a proper solution has been created? Thanks!

I've managed to reproduce this locally in a Docker container. It's a bug in the older binutils bfd ld, the __start_ and __stop_ have incorrect addresses, it's working correctly in gold and also in newer binutils versions. I'll send a change to conditionally enable this feature with -fbinutils-version=, I'm just trying to identify what version of binutils this was fixed in.

I have bisected this to binutils 2.28 and I have sent D97336 to conditionalize the use of nobits to binutils >=2.28.

[Conanap]

In D97110#2583184, @phosek wrote:

In D97110#2583082, @phosek wrote:

In D97110#2583026, @Conanap wrote:

Hello,

Power PC is also experiencing the same error,

Malformed instrumentation profile data
 error: No profiles could be merged.

I built a branch of main with this change reverted and the error goes away. Would it be possible to have this reverted for now until a proper solution has been created? Thanks!

I've managed to reproduce this locally in a Docker container. It's a bug in the older binutils bfd ld, the __start_ and __stop_ have incorrect addresses, it's working correctly in gold and also in newer binutils versions. I'll send a change to conditionally enable this feature with -fbinutils-version=, I'm just trying to identify what version of binutils this was fixed in.

I have bisected this to binutils 2.28 and I have sent D97336 to conditionalize the use of nobits to binutils >=2.28.

Awesome, thank you for the prompt response! I appreciate it.

[aeubanks]

In D97110#2583105, @davidxl wrote:

Kazu has seen this error

error: SHT_NOBITS section '__llvm_prf_cnts' cannot have non-zero
initializers

This is with lld.

David

I'm also seeing this in a bootstrapped PGO build.

$ ./tools/clang/scripts/build.py --llvm-force-head-revision --without-android --without-fuchsia --pgo --bootstrap
https://source.chromium.org/chromium/chromium/src/+/master:tools/clang/scripts/build.py?q=f:tools%2Fclang%2Fscripts%2Fbuild.py&ss=chromium

It appears using LLD after profile generation in the final optimized Clang build. e.g.

[72/3520] Building CXX object lib/Support/CMakeFiles/LLVMSupport.dir/ELFAttributeParser.cpp.o
<unknown>:0: error: SHT_NOBITS section '__llvm_prf_cnts' cannot have non-zero initializers

[aeubanks]

Sorry, it's actually while building the instrumented compiler, not the final optimized compiler.

[phosek]

I'm still debugging this, but it looks like counters for global constructors (_GLOBAL__sub_I_ symbols) aren't marked as zeroinitializer which looks like a bug to me.

[phosek]

The problem seems to be that Global Value Optimizer may evaluate and eliminate global initializer, and propagate the value to the counter symbol which is no longer zero initialized in that case. This is working as intended. Given that, we may have to change the logic to first ensure that the section content is all zeros before choosing the nobits type. I have reverted this change for now while I work on the implementation.

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

static SectionKind getELFKindForNamedSection(StringRef Name, SectionKind K) {
 
   if (Name == getInstrProfSectionName(IPSK_covmap, Triple::ELF,
                                       /*AddSegmentInfo=*/false) ||
       Name == getInstrProfSectionName(IPSK_covfun, Triple::ELF,
                                       /*AddSegmentInfo=*/false) ||
       Name == ".llvmbc" || Name == ".llvmcmd")
     return SectionKind::getMetadata();
 
+  if (Name == getInstrProfSectionName(IPSK_cnts, Triple::ELF,
+                                      /*AddSegmentInfo=*/false))
+    return SectionKind::getBSS();
+
   if (Name.empty() || Name[0] != '.') return K;
 
   // Default implementation based on some magic section names.
   if (Name == ".bss" ||
       Name.startswith(".bss.") ||
       Name.startswith(".gnu.linkonce.b.") ||
       Name.startswith(".llvm.linkonce.b.") ||
       Name == ".sbss" ||

llvm/test/Instrumentation/InstrProfiling/X86/nobits.ll

+;; Ensure that SHT_NOBITS section type is set for __llvm_prf_cnts in ELF.
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+
+@__profc_foo = hidden global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", align 8
+
+define void @foo() {
+  %pgocount = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0), align 4
+  %1 = add i64 %pgocount, 1
+  store i64 %1, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0), align 4
+  ret void
+}
+
+declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
+
+; CHECK: .section __llvm_prf_cnts,"aw",@nobits