[PATCH] D21405: [PGO] IRPGO pre-cleanup pass changes

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:216
@@ +215,3 @@
+    return 25;
+  return 75;
+}
----------------
mehdi_amini wrote:
> davidxl wrote:
> > Needs to have an option to control the value.
> Apparently `-inline-threshold` will override.
I don't think that is the right option to use. The -inline-threshold option is used to control the regular inliner -- the tuning option of the pre-cleanup should not affect it.

================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:228
@@ +227,3 @@
+    MPM.add(createSROAPass());
+    MPM.add(createEarlyCSEPass());             // Catch trivial redundancies
+    MPM.add(createCFGSimplificationPass());    // Merge & remove BBs
----------------
mehdi_amini wrote:
> mehdi_amini wrote:
> > Uh? Since we have Os/Oz as function attribute, it seems strange to me to have this hard-coded on a global level.
> Also is it duplicating `llvm::computeThresholdFromOptLevels` ?
It does not. The pre-inline's role is to do pre-profiling cleanup,  but to do full scale inlining, thus it needs to be more conservative (no profile guidance is available at this point).  It should not controlled in the same way as the regular inline phase.

================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:228
@@ +227,3 @@
+    MPM.add(createSROAPass());
+    MPM.add(createEarlyCSEPass());             // Catch trivial redundancies
+    MPM.add(createCFGSimplificationPass());    // Merge & remove BBs
----------------
davidxl wrote:
> mehdi_amini wrote:
> > mehdi_amini wrote:
> > > Uh? Since we have Os/Oz as function attribute, it seems strange to me to have this hard-coded on a global level.
> > Also is it duplicating `llvm::computeThresholdFromOptLevels` ?
> It does not. The pre-inline's role is to do pre-profiling cleanup,  but to do full scale inlining, thus it needs to be more conservative (no profile guidance is available at this point).  It should not controlled in the same way as the regular inline phase.
There is no mechanizm to do threshold tuning on a function granularity. Even there is one, using it may not help the goal of reducing the final overall size.


http://reviews.llvm.org/D21405