[llvm] d0b4780 - [nfc][docs]Generalize header description and ascii art for indexed profiles (#83507)

[via llvm-commits]

Author: Mingming Liu
Date: 2024-03-28T16:45:41-07:00
New Revision: d0b47806c01b27feefca2d39e8fc18d3ddec5f3c

URL: https://github.com/llvm/llvm-project/commit/d0b47806c01b27feefca2d39e8fc18d3ddec5f3c
DIFF: https://github.com/llvm/llvm-project/commit/d0b47806c01b27feefca2d39e8fc18d3ddec5f3c.diff

LOG: [nfc][docs]Generalize header description and ascii art for indexed profiles (#83507)

- Add pointers to code for source of truth.
- Move necessary details from doc to code.

Added: 
    

Modified: 
    llvm/docs/InstrProfileFormat.rst
    llvm/include/llvm/ProfileData/InstrProf.h

Removed: 
    


################################################################################
diff  --git a/llvm/docs/InstrProfileFormat.rst b/llvm/docs/InstrProfileFormat.rst
index 2069b87a245a13..ec1d1e2d7401fa 100644
--- a/llvm/docs/InstrProfileFormat.rst
+++ b/llvm/docs/InstrProfileFormat.rst
@@ -360,6 +360,10 @@ profiles generated by older tools or compilers.
 General Storage Layout
 -----------------------
 
+The ASCII art depicts the general storage layout of indexed profiles.
+Specifically, the indexed profile header describes the byte offset of individual
+payload sections.
+
 ::
 
                             +-----------------------+---+
@@ -369,55 +373,49 @@ General Storage Layout
                             +-----------------------+   |
                             |        HashType       |   H
                             +-----------------------+   E
-                    +-------|       HashOffset      |   A
-                    |       +-----------------------+   D
-                +-----------|     MemProfOffset     |   E
-                |   |       +-----------------------+   R
-                |   |    +--|     BinaryIdOffset    |   |
-                |   |    |  +-----------------------+   |
-            +---------------|      TemporalProf-    |   |
-            |   |   |    |  |      TracesOffset     |   |
-            |   |   |    |  +-----------------------+---+
-            |   |   |    |  |   Profile Summary     |   |
-            |   |   |    |  +-----------------------+   P
-            |   |   +------>|    Function data      |   A
-            |   |        |  +-----------------------+   Y
-            |   +---------->|  MemProf profile data |   L
-            |            |  +-----------------------+   O
-            |            +->|    Binary Ids         |   A
+                            |       Byte Offset     |   A
+                    +------ |      of section A     |   D
+                    |       +-----------------------+   E
+                    |       |       Byte Of fset    |   R
+                +-----------|      of section B     |   |
+                |   |       +-----------------------+   |
+                |   |       |         ...           |   |
+                |   |       +-----------------------+   |
+                |   |       |      Byte Offset      |   |
+            +---------------|     of section Z      |   |
+            |   |   |       +-----------------------+---+
+            |   |   |       |    Profile Summary    |   |
+            |   |   |       +-----------------------+   P
+            |   |   +------>|      Section A        |   A
+            |   |           +-----------------------+   Y
+            |   +---------->|      Section B        |   L
+            |               +-----------------------+   O
+            |               |         ...           |   A
             |               +-----------------------+   D
-            +-------------->|  Temporal profiles    |   |
+            +-------------->|      Section Z        |   |
                             +-----------------------+---+
 
-Header
---------
+.. note::
 
-``Magic``
-  The purpose of the magic number is to be able to tell if the profile is an
-  indexed profile.
+  Profile summary section is at the beginning of payload. It's right after the
+  header so its position is implicitly known after reading the header.
 
-``Version``
-  Similar to raw profile version, the lower 32 bits specify the version of the
-  indexed profile and the most significant 32 bits are reserved to specify the
-  variant types of the profile.
+Header
+--------
 
-``HashType``
-  The hashing scheme for on-disk hash table keys. Only MD5 hashing is used as of
-  writing.
+The `Header struct`_ is the source of truth and struct fields should explain
+what's in the header. At a high level, `*Offset` fields record section byte
+offsets, which are used by readers to locate interesting sections and skip
+uninteresting ones.
 
-``HashOffset``
-  An on-disk hash table stores the per-function profile records. This field records
-  the offset of this hash table's metadata (i.e., the number of buckets and
-  entries), which follows right after the payload of the entire hash table.
+.. note::
 
-``MemProfOffset``
-  Records the byte offset of MemProf profiling data.
+  To maintain backward compatibility of the indexed profiles, existing fields
+  shouldn't be deleted from struct definition; the field order shouldn't be
+  modified. New fields should be appended.
 
-``BinaryIdOffset``
-  Records the byte offset of binary id sections.
+.. _`Header struct`: https://github.com/llvm/llvm-project/blob/1a2960bab6381f2b288328e2371829b460ac020c/llvm/include/llvm/ProfileData/InstrProf.h#L1053-L1080
 
-``TemporalProfTracesOffset``
-  Records the byte offset of temporal profiles.
 
 Payload Sections
 ------------------

diff  --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 612c444faec648..f6c7960ed8bc5c 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -1062,9 +1062,15 @@ inline uint64_t ComputeHash(StringRef K) { return ComputeHash(HashType, K); }
 // as appropriate when updating the indexed profile format.
 struct Header {
   uint64_t Magic;
+  // The lower 32 bits specify the version of the indexed profile.
+  // The most significant 32 bits are reserved to specify the variant types of
+  // the profile.
   uint64_t Version;
   uint64_t Unused; // Becomes unused since version 4
   uint64_t HashType;
+  // This field records the offset of this hash table's metadata (i.e., the
+  // number of buckets and entries), which follows right after the payload of
+  // the entire hash table.
   uint64_t HashOffset;
   uint64_t MemProfOffset;
   uint64_t BinaryIdOffset;