[compiler-rt] [llvm] [TypeProf][InstrPGO] Introduce raw and instr profile format change for type profiling. (PR #81691)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 16 12:16:17 PST 2024


================
@@ -455,12 +455,12 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
   Header.MemProfOffset = 0;
   Header.BinaryIdOffset = 0;
   Header.TemporalProfTracesOffset = 0;
+  Header.VTableNamesOffset = 0;
   int N = sizeof(IndexedInstrProf::Header) / sizeof(uint64_t);
 
-  // Only write out all the fields except 'HashOffset', 'MemProfOffset',
-  // 'BinaryIdOffset' and `TemporalProfTracesOffset`. We need to remember the
-  // offset of these fields to allow back patching later.
-  for (int I = 0; I < N - 4; I++)
+  // Only write out the first four fields. We need to remember the offset of the
+  // remaining fields to allow back patching later.
+  for (int I = 0; I < 4; I++)
----------------
minglotus-6 wrote:

thanks for the comment @snehasish! While it turns out `compiler-rt/test/` has the test coverage we want [1], a change like this warrants a higher degree of caution.

@david-xl I wonder what's the reason we want to keep 'N - constant' here? I'm open to make the change, yet felt the current way saves a bump to `constant` if more fields are appended in the header struct later.
* We need patching since profile summary section comes right after header (and before hashing table). To maintain backward compatibility of the indexed profiles, we don't want to delete existing header fields or adjust their order.


[1] Change this to 5 causes failures in `compiler-rt/test`

https://github.com/llvm/llvm-project/pull/81691


More information about the llvm-commits mailing list