[PATCH] D68901: [SampleFDO] Add profile remapping support for profile on-demand loading used by ExtBinary format profile

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 15 11:45:33 PDT 2019


wmi added a comment.

Thanks Wenlei and David for the reviews.

It is a good suggestion to try to make remapper and reader less interleaving. That leads me to find a more fundamental problem. Current mechanism to make SampleProfileReaderItaniumRemapper a proxy of underlying reader is easy to have bugs. Currently, some internal data structures like Profiles and NameTables of underlyingReader have been transfered to SampleProfileReaderItaniumRemapper in order for SampleProfileReaderItaniumRemapper to function as a normal reader. However, for every virtual function in SampleProfileReader, we need to have an override function in SampleProfileReaderItaniumRemapper and delegate the function to the underlying reader, otherwise, it will call the wrong version in SampleProfileReader. It is confusing and it is easy to have bugs -- the underlying reader is ExtBinary format but SampleProfileReaderItaniumRemapper behaves like a SampleProfileReader.

So I plan to make SampleProfileReaderItaniumRemapper a helper inside SampleProfileReader instead of a proxy. From what I already saw that will simplify the code a lot. Will update the patch when it is done.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68901/new/

https://reviews.llvm.org/D68901





More information about the llvm-commits mailing list