[llvm-dev] Support for older format versions in RawInstrProfReader

[Xinliang David Li via llvm-dev]

On Tue, Aug 24, 2021 at 6:43 PM Petr Hosek <phosek at google.com> wrote:

> Using one llvm-profdata binary per-version is what we do today where our
> infrastructure scripts read the header from each profile, check the version
> and pick up the corresponding llvm-profdata binary. The challenge is mostly
> the overhead of managing these binaries and dispatching to the appropriate
> one based on the raw profile version (as opposed to simply taking all raw
> profiles and passing them to a single llvm-profdata binary which is what we
> used to do in the past).
>

This is kind of like making llvm-profdata an unbounded busy-box to handle
old versions.

>
> I agree that having the ability to drastically change the format is very
> valuable and it's definitely something I'd want to preserve. What I
> envisioned is rather than trying to support multiple versions inside the
> existing RawInstrProfReader class, we could have multiple classes and
> dispatch to the appropriate one based on the version. So essentially
> implementing what we do in our downstream scripts, but directly inside the
> llvm-profdata tool.
>
> The biggest advantage, aside from simplifying our own infrastructure, is
> the fact that this logic could be reused across different llvm-profdata
> users, but I don't know if there are other users that have the same issue.
> The disadvantage is mostly the increased complexity compared to the
> existing implementation and potentially some code duplication.
>

Overtime, the tool will become very bloated. Setting up a limit on the
number of versions supported can avoid the problem but the limit itself is
very arbitrary. I still feel that asking users to stash the old
llvm-profdata to handle older versions is the right way to go.


David



>
> On Tue, Aug 24, 2021 at 1:38 PM Vedant Kumar <vsk at apple.com> wrote:
>
>>
>>
>> On Aug 24, 2021, at 8:24 AM, Xinliang David Li <davidxl at google.com>
>> wrote:
>>
>> On Mon, Aug 23, 2021 at 10:52 PM Petr Hosek <phosek at google.com> wrote:
>>
>>> Currently, while IndexedInstrProfReader provides backwards compatibility
>>> for indexed profile format, RawInstrProfReader only supports the current
>>> version of raw profile format. Has support for older raw profile format
>>> versions in RawInstrProfReader ever been considered?
>>>
>>
>> Justin (cc'd) once explained to me that a key motivating factor for
>> splitting up the raw and indexed profile formats was to free the runtime
>> from back-compat concerns.
>>
>>
>>> The reason why I'm asking is that we collect code coverage from both C++
>>> and Rust, but since Clang and Rust toolchains use different version on
>>> LLVM, we need to match each profile with a corresponding llvm-profdata
>>> binary which has been increasing the complexity of our infrastructure quite
>>> a bit.
>>>
>>
>> Is it possible to save one reader binary per raw format version? What is
>> the challenge of matching up versions with the reader binary?
>>
>>>
>>> Using the same llvm-profdata tool would simplify things a lot but would
>>> require backwards compatibility support in RawInstrProfReader. Would that
>>> be desirable?
>>>
>>
>> Current scheme for raw format allows fast pace change and innovation. It
>> also allows quite drastical format change. Losing that capability would be
>> undesirable. Before pursuing this path, the benefit needs to outweigh the
>> cost.
>>
>>
>> + 1
>>
>> vedant
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210826/dd3f8a07/attachment.html>