[PATCH] D81682: [PGO] Extend the value profile buckets for mem op sizes.

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

the approach looks sound.



================
Comment at: compiler-rt/include/profile/InstrProfData.inc:160
 #else /* VALUE_RANGE_PROF */
-VALUE_PROF_FUNC_PARAM(uint32_t, CounterIndex, Type::getInt32Ty(Ctx)) \
-                      INSTR_PROF_COMMA
-VALUE_PROF_FUNC_PARAM(uint64_t, PreciseRangeStart, Type::getInt64Ty(Ctx)) \
-                      INSTR_PROF_COMMA
-VALUE_PROF_FUNC_PARAM(uint64_t, PreciseRangeLast, Type::getInt64Ty(Ctx)) \
-                      INSTR_PROF_COMMA
-VALUE_PROF_FUNC_PARAM(uint64_t, LargeValue, Type::getInt64Ty(Ctx))
+                          /* This is to be removed after switching to the new
+                             memop value profiling. */
----------------
add FIXME


================
Comment at: compiler-rt/include/profile/InstrProfData.inc:162
+                             memop value profiling. */
+                          VALUE_PROF_FUNC_PARAM(uint32_t, CounterIndex,
+                                                Type::getInt32Ty(Ctx))
----------------
unwanted format change?


================
Comment at: compiler-rt/include/profile/InstrProfData.inc:813
+ *   [4, 4]
+ *   [5, 5]
+ *   [6, 6]
----------------
should 5,6, and 7 be merged into one range?


================
Comment at: compiler-rt/lib/profile/InstrProfilingValue.c:254
  */
+/* This is to be removed after switching to the new memop value profiling. */
 COMPILER_RT_VISIBILITY void __llvm_profile_instrument_range(
----------------
add FIXME


================
Comment at: llvm/include/llvm/ProfileData/InstrProf.h:78
 /// Return the name profile runtime entry point to do value range profiling.
+// This is to be removed after switching to the new memop value profiling.
 inline StringRef getInstrProfValueRangeProfFuncName() {
----------------
FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81682