[compiler-rt] [llvm] [InstrFDO]Allow indexed profile reader to parse compatible future versions and returns errors for incompatible ones. (PR #88212)

[Mingming Liu via llvm-commits]

minglotus-6 wrote:

> I think the on-disk header size is very useful. People can keep adding new sections without affecting others who are not interested in the new sections.
> 
> I am not to sure about the minimum compatible version. I have a couple of concerns:
> 
> * When somebody bumps the minimum compatible version due to an update to one specific section, they would teach `InstrProfWriter::minCompatibleReaderVersion` to return `Version14` or some higher number.  This is a profile-wide number, not the version of some specific section.  This means that users lose the forward compatibility regardless of whether they are using the updated section.

Currently many payload sections don't have a sub-section version and indexed profile format doesn't have any forward compatibility, I think introducing profile-wide min compatible version is simpler than backfilling versions for every payload sections, what do you think?

I agree it's a valid use case to update version of specific payload sections. If the payload section have versions (e.g., memory profiles), profile reader can do the version check on sub-sections and there is no need to bump profile-wide versions.

>  In any event, there should be some documentation, either as comments in the code or some .rst, about how to add and modify a section. Relevant functions and variables should point to the documentation.

Good point about documentation update. Comments have closer proximity to code for details but doc can summarize high-level ideas. I have https://llvm.org/docs/InstrProfileFormat.html an https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#format-compatibility-guarantees in mind for now. Will update the pr.


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