[llvm] Implements PGOBBAddrMap in Object and ObjectYAML with tests [1/5] (PR #71750)

Micah Weston via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 15 07:51:58 PST 2023


red1bluelost wrote:

> > If we keep the two classes `PGOBBAddrMap` and `BBAddrMap`, I don't think that this would simplify the encoding/decoding logic that much. `PGOBBAddrMap` would be the same while `BBAddrMap` would either need to error if Feature is not zero or decode but ignore the extra data. I personally would lean towards having `BBAddrMap` ignore extra data since `PGO...` is a superset and I suspect ignoring could simplify 'trait design' complexity.
> 
> One way to do this is to have the decoding function return two structures `std::vector<BBAddrMap>` and `std::vector<PGOFrequencyMap>`. `PGOFrequencyMap` does not store any address information so we don't need to duplicate the base fields. The only drawback is that we need to define the mapping between the two data structures.
> 
> For decoding, we can inspect the feature field and populate `PGOFrequencyMap` if we need to. WDYT?

I see. I think that should be better. Using the "PGOFrequencyMap" will definitely help simplify Object's decode/encode. ObjectYAML is the harder area to tackle. I have one more idea to try so that we don't need new fields in yaml::BBAddrMapEntry, but if it fails, it might be cleanest to duplicate the fields just in yaml.

I think mapping should be fine. I see two designs.
1. We could just keep the sizes identical to have one-to-one mapping. The PGOFrequencyMap would contain FuncEntryCount and a vector of BBFreq and BrProbs. In the case where a function is cold or empty, we could just leave the block vector empty.
2. We could include the function address in PGOFrequencyMap and not make cold or empty function. I'm not sure if there on concerns with unique addresses.

I'll try building the first designs.

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


More information about the llvm-commits mailing list