[PATCH] D59514: [PGO] Profile guided code size optimization.

[Easwaran Raman via Phabricator via llvm-commits]

eraman added inline comments.


================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:331
+
+bool ProfileSummaryInfo::shouldOptimizeForSize(Function *F, BasicBlock *BB,
+                                               BlockFrequencyInfo *BFI) {
----------------
This function looks very odd to me. Why not separate it into shouldOptimizeForSize(Function *, BFI) and shouldOptimizeForSize(BasicBlock*,...)? All but one uses of this function passes a BB in which case the function is irrrelevant.


================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:62
+    "pgso", cl::Hidden, cl::init(true),
+    cl::desc("Enable the profile guided size optimization. "));
+
----------------
Perhaps better to default to false, do robust performance testing and flip the option. You've mentioned code size reduction in one binary in the description, but good to validate there are no  performance regressions in spec.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7139
   // Check the function attributes to find out if this function should be
   // optimized for size.
   bool OptForSize =
----------------
Comment out of sync with the  code


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7223
   // Check the function attributes to find out if this function should be
   // optimized for size.
   bool OptForSize =
----------------
Augment the comment to make it in-sync with the code


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59514