[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