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

[David Li via Phabricator via llvm-commits]

davidxl 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,
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73381