[PATCH] D73381: [PGO][PGSO] Handle MBFIWrapper

[Hiroshi Yamauchi via Phabricator via llvm-commits]

yamauchi added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/SizeOpts.h:92
+template<typename AdapterT, typename BFIT>
+bool shouldOptimizeForSizeImpl(BlockFrequency BlockFreq,
+                               ProfileSummaryInfo *PSI, BFIT *BFI,
----------------
davidxl wrote:
> yamauchi wrote:
> > davidxl wrote:
> > > Is this function necessary? It is mostly the same as the previous impl.
> > This version takes a BlockFrequency rather than a block (BlockT). Since only MBFIWrapper, rather than BFI, has the true frequency for the block, we'd need to either (1) get the frequency out of MBFIWrapper and then pass it here, or (2) pass MBFIWrapper down here. The current approach is (1). Either way, we'd need to take the frequency out of MBFIWrapper somwhere along the way, and it won't likely dedup the code as they both require a new param type that doesn't seem to be amenable to additional templatization on top of this already templatized code.
> > 
> > Do you see a better idea?
> How about changing BlockT template parameter into BlockTOrBlockFreq, so that it can be instantiated with BlockFrequency too?
That just worked out, except for the assert on the BB pointer non-nullness, which I resolved by moving it to the callers. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73381