[PATCH] D52056: [PGO][CHR] Add opt remarks.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/Transforms/Instrumentation/ControlHeightReduction.cpp:1171
+                                            RI.R->getEntry()->getTerminator())
+                << "Split scope from outer";
+          });
----------------
Can you make the missed reason string clearer?


================
Comment at: lib/Transforms/Instrumentation/ControlHeightReduction.cpp:1206
+                                          RI.R->getEntry()->getTerminator())
+              << "Split scope from previous";
+        });
----------------
Similarly here.


================
Comment at: lib/Transforms/Instrumentation/ControlHeightReduction.cpp:1331
+        str_ostream OS;
+        OS << "Drop scope with < " << CHRMergeThreshold
+           << " biased branch(es) or select(s)";
----------------
You can pass the argument to the ORE object with ore::NV type:

OptimizationMarkMissed ORE(DEBUG_TYPE, ....);
ORE << "Drop scope with < " << ore::NV("CHRMergeThreshold", CHRMergeThreshold) << ...


================
Comment at: lib/Transforms/Instrumentation/ControlHeightReduction.cpp:1831
+    str_ostream OS;
+    OS << "Merged " << NumCHRedBranches << " branches or selects";
+    return OptimizationRemark(DEBUG_TYPE,
----------------
use ore::NV to pass argument.


================
Comment at: lib/Transforms/Instrumentation/ControlHeightReduction.cpp:2031
+      OS << F.getName() << " "
+         << F.getParent()->getName() << " "
+         << "Reduced the number of branches in hot paths by "
----------------
The message already includes the module name, so there is no need to do it here.

Also the function name can be passed as NV:

ORE<< ore::NV("Function", &F) << "Reduced .... by " << ore::NV("NumBranchesDelta", Stats.NumBranchesDelta) << " (static ) ...." ...


Repository:
  rL LLVM

https://reviews.llvm.org/D52056