Page MenuHomePhabricator

[ThinLTO] Represent relative BF using a scaled representation .

Authored by eraman on Feb 20 2018, 11:35 AM.



The current integer representation of relative block frequency prevents
representing relative block frequencies below 1. This change uses a 8 of
the 29 bits to represent the decimal part by using a fixed scale of -8.

Diff Detail


Event Timeline

eraman created this revision.Feb 20 2018, 11:35 AM
davidxl added inline comments.Feb 20 2018, 1:18 PM
63 ↗(On Diff #135116)

Document that it is RelativeBlockFrequency scaled by 2^8

Also should all the uses of RelBlockFreq scale it back?

65 ↗(On Diff #135116)

Should this be scaled up too?

86 ↗(On Diff #135116)

EntryFreq can be zero.

eraman marked 2 inline comments as done.Feb 20 2018, 5:31 PM
eraman added inline comments.
63 ↗(On Diff #135116)


Yes, users should scale it back. In a separate patch, I will add a method to get this value as a ScaledNumber or uint64_t so that the users don't have to explicitly scale them

65 ↗(On Diff #135116)

No, this is just the max value of the digits in the 29 bit bit-field.

86 ↗(On Diff #135116)

Somehow missed this in refactoring.

eraman updated this revision to Diff 135178.Feb 20 2018, 5:32 PM
eraman marked 2 inline comments as done.

Address review comments.

davidxl added inline comments.Feb 20 2018, 9:30 PM
65 ↗(On Diff #135116)

then the 'min' function here Sum = std::min(Sum, uint64_t(MaxRelBlockFreq)); should use a scaled MaxRelBlockFreq, right?

eraman added inline comments.Feb 21 2018, 12:34 PM
65 ↗(On Diff #135116)

No. The intent is to use a max of 29 bits including the integer and decimal(fractional) bits. Since I use a scale of 8, 8 bits will be used to represent values below 1 and I can use only 21 bits to represent the integral part.

This revision is now accepted and ready to land.Feb 21 2018, 12:45 PM
This revision was automatically updated to reflect the committed changes.