[Profile] Use upper 32 bits of profile version for profile variants.

[ZequanWu]

Currently all upper 8 bits are reserved for different profile variants. We need more bits for new mods in the future.
Context: https://discourse.llvm.org/t/how-to-add-a-new-mode-to-llvm-raw-profile-version/73688

[llvmbot]

@llvm/pr-subscribers-pgo

Changes

Currently all upper 8 bits are reserved for different profile variants. We need more bits for new mods in the future.
Context: https://discourse.llvm.org/t/how-to-add-a-new-mode-to-llvm-raw-profile-version/73688

---

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

5 Files Affected:

• (modified) compiler-rt/include/profile/InstrProfData.inc (+2-3)
• (modified) llvm/include/llvm/ProfileData/InstrProfData.inc (+2-3)
• (modified) llvm/include/llvm/ProfileData/InstrProfReader.h (+2-2)
• (modified) llvm/lib/ProfileData/InstrProfReader.cpp (+1-1)
• (modified) llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test (+2-2)

diff --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
index 4456bf1ab176325..a000a38098ee30b 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -644,7 +644,6 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
        (uint64_t)'p' << 40 | (uint64_t)'r' << 32 | (uint64_t)'o' << 24 |  \
         (uint64_t)'f' << 16 | (uint64_t)'R' << 8 | (uint64_t)129
 
-/* FIXME: Please remedy the fixme in the header before bumping the version. */
 /* Raw profile format version (start from 1). */
 #define INSTR_PROF_RAW_VERSION 8
 /* Indexed profile format version (start from 1). */
@@ -652,7 +651,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 /* Coverage mapping format version (start from 0). */
 #define INSTR_PROF_COVMAP_VERSION 5
 
-/* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
+/* Profile version is always of type uint64_t. Reserve the upper 32 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
  * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentation
  * generated profile, and 0 if this is a Clang FE generated profile.
@@ -663,7 +662,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
  * The 62nd bit indicates whether memory profile information is present.
  * The 63rd bit indicates if this is a temporal profile.
  */
-#define VARIANT_MASKS_ALL 0xff00000000000000ULL
+#define VARIANT_MASKS_ALL 0xffffffff00000000ULL
 #define GET_VERSION(V) ((V) & ~VARIANT_MASKS_ALL)
 #define VARIANT_MASK_IR_PROF (0x1ULL << 56)
 #define VARIANT_MASK_CSIR_PROF (0x1ULL << 57)
diff --git a/llvm/include/llvm/ProfileData/InstrProfData.inc b/llvm/include/llvm/ProfileData/InstrProfData.inc
index 4456bf1ab176325..a000a38098ee30b 100644
--- a/llvm/include/llvm/ProfileData/InstrProfData.inc
+++ b/llvm/include/llvm/ProfileData/InstrProfData.inc
@@ -644,7 +644,6 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
        (uint64_t)'p' << 40 | (uint64_t)'r' << 32 | (uint64_t)'o' << 24 |  \
         (uint64_t)'f' << 16 | (uint64_t)'R' << 8 | (uint64_t)129
 
-/* FIXME: Please remedy the fixme in the header before bumping the version. */
 /* Raw profile format version (start from 1). */
 #define INSTR_PROF_RAW_VERSION 8
 /* Indexed profile format version (start from 1). */
@@ -652,7 +651,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 /* Coverage mapping format version (start from 0). */
 #define INSTR_PROF_COVMAP_VERSION 5
 
-/* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
+/* Profile version is always of type uint64_t. Reserve the upper 32 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
  * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentation
  * generated profile, and 0 if this is a Clang FE generated profile.
@@ -663,7 +662,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
  * The 62nd bit indicates whether memory profile information is present.
  * The 63rd bit indicates if this is a temporal profile.
  */
-#define VARIANT_MASKS_ALL 0xff00000000000000ULL
+#define VARIANT_MASKS_ALL 0xffffffff00000000ULL
 #define GET_VERSION(V) ((V) & ~VARIANT_MASKS_ALL)
 #define VARIANT_MASK_IR_PROF (0x1ULL << 56)
 #define VARIANT_MASK_CSIR_PROF (0x1ULL << 57)
diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
index faf91e017756da9..17194e2aa4c73ca 100644
--- a/llvm/include/llvm/ProfileData/InstrProfReader.h
+++ b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -318,8 +318,8 @@ class RawInstrProfReader : public InstrProfReader {
   /// A list of timestamps paired with a function name reference.
   std::vector<std::pair<uint64_t, uint64_t>> TemporalProfTimestamps;
   bool ShouldSwapBytes;
-  // The value of the version field of the raw profile data header. The lower 56
-  // bits specifies the format version and the most significant 8 bits specify
+  // The value of the version field of the raw profile data header. The lower 32
+  // bits specifies the format version and the most significant 32 bits specify
   // the variant types of the profile.
   uint64_t Version;
   uint64_t CountersDelta;
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 9375678f8046167..125184d4020840d 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -38,7 +38,7 @@
 
 using namespace llvm;
 
-// Extracts the variant information from the top 8 bits in the version and
+// Extracts the variant information from the top 32 bits in the version and
 // returns an enum specifying the variants present.
 static InstrProfKind getProfileKindFromVersion(uint64_t Version) {
   InstrProfKind ProfileKind = InstrProfKind::Unknown;
diff --git a/llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test b/llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test
index 24f3f563e9689d6..c0072bcbde1b387 100644
--- a/llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test
+++ b/llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test
@@ -1,7 +1,7 @@
 // Magic
 RUN: printf '\377lprofr\201' > %t
 // Version
-RUN: printf '\0\01\0\0\0\0\0\10' >> %t
+RUN: printf '\0\0\0\0\10\0\0\10' >> %t
 // The rest of the header needs to be there to prevent a broken header error.
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
@@ -15,5 +15,5 @@ RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 
 RUN: not llvm-profdata show %t -o /dev/null 2>&1 | FileCheck %s
 
-CHECK: raw profile version mismatch: Profile uses raw profile format version = 281474976710664; expected version = {{[0-9]+}}
+CHECK: raw profile version mismatch: Profile uses raw profile format version = 134217736; expected version = {{[0-9]+}}
 CHECK-NEXT: PLEASE update this tool to version in the raw profile, or regenerate raw profile with expected version.

[ZequanWu]

I removed this because it was already fixed at https://reviews.llvm.org/D158466

[petrhosek]

We could also consider just using 16 bits for flags rather than going all the way to 32, but I don't have a strong preference, either way is fine with me.

[david-xl]

32 bits or 16 bits are fine to me. Wait for @xur-llvm to chime in.

[xur-llvm]

I'm also fine with using more bits for profile variants information. 32 bits seem a lot as of now. But we don't need that many bits for versions. So 32-bit is fine with me.

[xur-llvm]

Please fix the typos.