[profile] Change __llvm_profile_counter_bias etc. types to match llvm

[rorth]

As detailed in Issue #101667, two profile tests FAIL on 32-bit SPARC, both Linux/sparc64 and Solaris/sparcv9 (where the tests work when enabled):

  Profile-sparc :: ContinuousSyncMode/runtime-counter-relocation.c
  Profile-sparc :: ContinuousSyncMode/set-file-object.c

The Solaris linker provides the crucial clue as to what's wrong:

ld: warning: symbol '__llvm_profile_counter_bias' has differing sizes:
	(file runtime-counter-relocation-17ff25.o value=0x8; file libclang_rt.profile-sparc.a(InstrProfilingFile.c.o) value=0x4);
	runtime-counter-relocation-17ff25.o definition taken

In fact, the types in llvm and compiler-rt differ:

• __llvm_profile_counter_bias/INSTR_PROF_PROFILE_COUNTER_BIAS_VAR is created in llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp (InstrLowerer::getCounterAddress) as int64_t, while compiler-rt/lib/profile/InstrProfilingFile.c uses intptr_t. While this doesn't matter in the 64-bit case, the type sizes differ for 32-bit.
• __llvm_profile_bitmap_bias/INSTR_PROF_PROFILE_BITMAP_BIAS_VAR has the same issue: created in InstrProfiling.cpp (InstrLowerer::getBitmapAddress) as int64_t, while InstrProfilingFile.c again uses intptr_t.

This patch changes the compiler-rt types to match llvm. At the same time, the affected testcases are enabled on Solaris, too, where they now just PASS.

Tested on sparc64-unknown-linux-gnu, sparcv9-sun-solaris2.11, x86_64-pc-linux-gnu, and `amd64-pc-solaris2.11.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Rainer Orth (rorth)

Changes

As detailed in Issue #101667, two profile tests FAIL on 32-bit SPARC, both Linux/sparc64 and Solaris/sparcv9 (where the tests work when enabled):

  Profile-sparc :: ContinuousSyncMode/runtime-counter-relocation.c
  Profile-sparc :: ContinuousSyncMode/set-file-object.c

The Solaris linker provides the crucial clue as to what's wrong:

ld: warning: symbol '__llvm_profile_counter_bias' has differing sizes:
	(file runtime-counter-relocation-17ff25.o value=0x8; file libclang_rt.profile-sparc.a(InstrProfilingFile.c.o) value=0x4);
	runtime-counter-relocation-17ff25.o definition taken

In fact, the types in llvm and compiler-rt differ:

• __llvm_profile_counter_bias/INSTR_PROF_PROFILE_COUNTER_BIAS_VAR is created in llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp (InstrLowerer::getCounterAddress) as int64_t, while compiler-rt/lib/profile/InstrProfilingFile.c uses intptr_t. While this doesn't matter in the 64-bit case, the type sizes differ for 32-bit.
• __llvm_profile_bitmap_bias/INSTR_PROF_PROFILE_BITMAP_BIAS_VAR has the same issue: created in InstrProfiling.cpp (InstrLowerer::getBitmapAddress) as int64_t, while InstrProfilingFile.c again uses intptr_t.

This patch changes the compiler-rt types to match llvm. At the same time, the affected testcases are enabled on Solaris, too, where they now just PASS.

Tested on sparc64-unknown-linux-gnu, sparcv9-sun-solaris2.11, x86_64-pc-linux-gnu, and `amd64-pc-solaris2.11.

---

Full diff: https://github.com/llvm/llvm-project/pull/102747.diff

4 Files Affected:

• (modified) compiler-rt/lib/profile/InstrProfilingFile.c (+6-6)
• (modified) compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c (+1-1)
• (modified) compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c (+1-1)
• (modified) compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c (+1-1)

diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index db3918d8410319..62af96331bc99d 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -200,16 +200,16 @@ static int mmapForContinuousMode(uint64_t CurrentFileOffset, FILE *File) {
 
 #define INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR                            \
   INSTR_PROF_CONCAT(INSTR_PROF_PROFILE_COUNTER_BIAS_VAR, _default)
-COMPILER_RT_VISIBILITY intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR = 0;
+COMPILER_RT_VISIBILITY int64_t INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR = 0;
 #define INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR                             \
   INSTR_PROF_CONCAT(INSTR_PROF_PROFILE_BITMAP_BIAS_VAR, _default)
-COMPILER_RT_VISIBILITY intptr_t INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR = 0;
+COMPILER_RT_VISIBILITY int64_t INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR = 0;
 
 /* This variable is a weak external reference which could be used to detect
  * whether or not the compiler defined this symbol. */
 #if defined(_MSC_VER)
-COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
-COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_BITMAP_BIAS_VAR;
+COMPILER_RT_VISIBILITY extern int64_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
+COMPILER_RT_VISIBILITY extern int64_t INSTR_PROF_PROFILE_BITMAP_BIAS_VAR;
 #if defined(_M_IX86) || defined(__i386__)
 #define WIN_SYM_PREFIX "_"
 #else
@@ -224,10 +224,10 @@ COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_BITMAP_BIAS_VAR;
                 INSTR_PROF_PROFILE_BITMAP_BIAS_VAR) "=" WIN_SYM_PREFIX         \
                 INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR))
 #else
-COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR
+COMPILER_RT_VISIBILITY extern int64_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR
     __attribute__((weak, alias(INSTR_PROF_QUOTE(
                              INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR))));
-COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_BITMAP_BIAS_VAR
+COMPILER_RT_VISIBILITY extern int64_t INSTR_PROF_PROFILE_BITMAP_BIAS_VAR
     __attribute__((weak, alias(INSTR_PROF_QUOTE(
                              INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR))));
 #endif
diff --git a/compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c b/compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c
index fdcb82e4d72baf..65b7bdaf403da4 100644
--- a/compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c
+++ b/compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c
@@ -35,7 +35,7 @@
 #include "InstrProfilingUtil.h"
 
 /* This variable is an external reference to symbol defined by the compiler. */
-COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
+COMPILER_RT_VISIBILITY extern int64_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
 
 COMPILER_RT_VISIBILITY unsigned lprofProfileDumped(void) {
   return 1;
diff --git a/compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c b/compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
index 4ca8bf62455371..19a7aae70cb0d3 100644
--- a/compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
+++ b/compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
@@ -1,4 +1,4 @@
-// REQUIRES: linux || windows
+// REQUIRES: target={{.*(linux|solaris|windows-msvc).*}}
 
 // RUN: %clang -fprofile-instr-generate -fcoverage-mapping -mllvm -runtime-counter-relocation=true -o %t.exe %s
 // RUN: echo "garbage" > %t.profraw
diff --git a/compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c b/compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c
index b52324d7091eb2..53609f5838f753 100644
--- a/compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c
+++ b/compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c
@@ -1,4 +1,4 @@
-// REQUIRES: darwin || linux
+// REQUIRES: target={{.*(darwin|linux|solaris).*}}
 
 // Test using __llvm_profile_set_file_object in continuous mode (%c).
 // Create & cd into a temporary directory.

[petrhosek]

I'd prefer fixing this in the other direction, that is changing InstrLowerer::getCounterAddress to use intptr_t. The bias is an offset from an address so there's no need to use int64_t on a 32-bit system.

[rorth]

I'd prefer fixing this in the other direction, that is changing InstrLowerer::getCounterAddress to use intptr_t. The bias is an offset from an address so there's no need to use int64_t on a 32-bit system.

While that is certainly an option, it has several drawbacks at this point:

• IIUC it would require creating a new profile version and adding support for reading that in addition to the current one.
• That's certainly a larger change and I wonder if it would be appropriate for backporting to the 19.x branch.
• I doubt I'm up to implementing that, while my proposed patch is just fixing the current format.

[tru]

Since this has some questions marks still and we are soon at final, do we want to handle this somehow?

[tru]

ping

[rorth]

I remain sceptical wrt. @petrhosek suggestion: while my patch trivially gets clang and compiler-rt in sync, his incompatibly changes the profile file format (for 32-bit targets) and requires considerably more effort to implement. Even if it could be implemented quickly (and I won't be able to do so), it's way too late and intrusive for the 19.x release. Maybe one of the other reviewers could chime in?

[petrhosek]

I remain sceptical wrt. @petrhosek suggestion: while my patch trivially gets clang and compiler-rt in sync, his incompatibly changes the profile file format (for 32-bit targets) and requires considerably more effort to implement. Even if it could be implemented quickly (and I won't be able to do so), it's way too late and intrusive for the 19.x release. Maybe one of the other reviewers could chime in?

That's a fair point, I think it's too late to make such a large change for the 19.x release so I'm fine merging this change despite the potential performance regression (although that's a pre-existing issue), but I think we should file an issue to implement the proper solution after.

[rorth]

/cherry-pick 70a19ad

[llvmbot]

Failed to cherry-pick: 70a19ad

https://github.com/llvm/llvm-project/actions/runs/10682962180

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

[rorth]

I've now created PR #107362 for the release/19.x branch backport. cherry-pick didn't work because the branch lacks __llvm_profile_bitmap_bias.