[PATCH] D89524: [NFC][SampleFDO] Move some common stuff from SampleProfileReaderExtBinary/SampleProfileWriterExtBinary to their parent classes.

Wenlei He via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 20 23:31:29 PDT 2020


wenlei added a comment.

Thanks for the context! So the intention is to use subclass to hide extra complexity of uncommon sections/variants and its reader/writer into subclass. Then this refactoring makes sense.

Just to confirm, this use case you described is the partial profile that was introduced a while ago, correct? And new variant will have new sections for flattened and hierarchical profile?

For `SecProfileSymbolList` specifically, I think it's common enough that justifies it to be in the base.

> Surely we also don't want to have too many classes either. So maybe we can support multiple layouts for a certain subclass if the layouts are similar. Only if the new variant is significantly different from existing one, we create a new class. What do you think?

Sounds good to me.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D89524



More information about the llvm-commits mailing list