[PATCH] D19404: Reorganize GlobalValueSummary with a "Flags" bitfield.

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 22 12:10:31 PDT 2016


tejohnson added a comment.

In http://reviews.llvm.org/D19404#409351, @joker.eph wrote:

> In http://reviews.llvm.org/D19404#409074, @tejohnson wrote:
>
> > The biggest potential issue seems to me the second disadvantage. Particularly if the a 0 value for the new flag allows us to be more aggressive in certain cases than we were with the older bitcode version that didn't have the flag. Any thoughts on how we could mitigate this risk? Otherwise, it seems like it might be more robust to explicitly list each flag in the bitcode format.
>
>
> You're right. We need to version the record somehow.
>  Even with separate fields with have the issue. Usually we can work around it with separate field because the number of entries in the record tells us which "version" we have. Here we can't do that for every record because some record have variable-length entries.
>  Something that could work is to have `[ ..., FlagsVersion (VBR6), Flags (VBR6), ...]` with `FlagsVersion` an integer incremented every time we make a change to the `Flags` content.


I like the version idea. However, instead of bloating out every single summary with this info, since all summaries should have the same version, I think we should put a single summary version id record at the start the summary section (not specific to the flags, since we will want to use this to detect other summary changes too).


http://reviews.llvm.org/D19404





More information about the llvm-commits mailing list