[PATCH] D15828: [PGO] Passmanagerbuilder change that enable IR level PGO instrumentation

[Sean Silva via llvm-commits]

silvas added inline comments.

================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:216
@@ +215,3 @@
+    MPM.add(createGVNPass(DisableGVNLoadPRE));  // Remove redundancies
+    addExtensionsToPM(EP_Peephole, MPM);
+  }
----------------
Let's drop the pre-instrumentation passes for now and leave a TODO. We will want to have a focused discussion on what to put here.

================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:281
@@ -239,1 +280,3 @@
 
+  addPGOInstrPasses(MPM);
+
----------------
> Yes, and there was not consensus in the RFC for the pre-inlining. Instead, the approach suggested was to work on independent components which seem non-controversial and have a separate focused discussion about pre-inlining and other interactions.

Agreed. We will definitely want to have a focused discussion with real-world performance measurements when doing pass pipeline tuning (I have a big chunk of time allocated for characterizing and tuning the new IR-level instrumentation stuff and am looking forward to participating!). For the moment, just wiring it up and leaving a TODO for the pre-instrumentation passes is probably the right incremental step.

(I just got back from vacation and am still working on my email backlog, but I should be back on my feet next week and the new IR-level instrumentation stuff is my top priority at the moment)


http://reviews.llvm.org/D15828