[PATCH] D54175: [PGO] context sensitive PGO

Mon Dec 3 12:23:47 PST 2018

xur marked an inline comment as done.
xur added a comment.

As for the tests, I meant to add test cases to this patch later, not through a different patch.
I can add tests now.

================
Comment at: lib/LTO/LTOBackend.cpp:284
   PMB.PGOSampleUse = Conf.SampleProfile;
+  PMB.EnablePGOCSInstrGen = Conf.RunCSIRInstr;
+  if (!Conf.CSIRProfile.empty()) {
----------------
tejohnson wrote:
> Is it deliberate that here in the old PM the RunCSIRInstr and CSIRProfile are handled regardless of whether SampleProfile is set, whereas in the new PM they are only handled when SampleProfile is not set? What should be the behavior if sample profile is specified along with CS profiling?
It's not deliberate. I should have make the behavior consistent. 
I think I would like to change new PM and make CSProfile independent of SampleProfile.
The reason is that -fprofile-use and -fprofile-sample-use are independent. 
This usage does not make a lot of sense, and we should probably give some warnings. But that should be in command line handling, not here.

What do you think?

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

https://reviews.llvm.org/D54175