[llvm] 27650ec - Revert D81682 "[PGO] Extend the value profile buckets for mem op sizes."

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 22 16:08:35 PDT 2020


Author: Fangrui Song
Date: 2020-07-22T16:08:25-07:00
New Revision: 27650ec5541cd604a5027ad63895e0badfd35efe

URL: https://github.com/llvm/llvm-project/commit/27650ec5541cd604a5027ad63895e0badfd35efe
DIFF: https://github.com/llvm/llvm-project/commit/27650ec5541cd604a5027ad63895e0badfd35efe.diff

LOG: Revert D81682 "[PGO] Extend the value profile buckets for mem op sizes."

This reverts commit 4a539faf74b9b4c25ee3b880e4007564bd5139b0.

There is a __llvm_profile_instrument_range related crash in PGO-instrumented clang:

```
(gdb) bt
llvm::ConstantRange const&, llvm::APInt const&, unsigned int, bool) ()
llvm::ScalarEvolution::getRangeForAffineAR(llvm::SCEV const*, llvm::SCEV
const*, llvm::SCEV const*, unsigned int) ()
```

(The body of __llvm_profile_instrument_range is inlined, so we can only find__llvm_profile_instrument_target in the trace)

```
 23│    0x000055555dba0961 <+65>:    nopw   %cs:0x0(%rax,%rax,1)
 24│    0x000055555dba096b <+75>:    nopl   0x0(%rax,%rax,1)
 25│    0x000055555dba0970 <+80>:    mov    %rsi,%rbx
 26│    0x000055555dba0973 <+83>:    mov    0x8(%rsi),%rsi  # %rsi=-1 -> SIGSEGV
 27│    0x000055555dba0977 <+87>:    cmp    %r15,(%rbx)
 28│    0x000055555dba097a <+90>:    je     0x55555dba0a76 <__llvm_profile_instrument_target+342>
```

Added: 
    

Modified: 
    compiler-rt/include/profile/InstrProfData.inc
    compiler-rt/lib/profile/InstrProfilingValue.c
    llvm/include/llvm/ProfileData/InstrProf.h
    llvm/include/llvm/ProfileData/InstrProfData.inc
    llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
    llvm/lib/ProfileData/InstrProf.cpp
    llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
    llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
    llvm/test/Transforms/PGOProfile/memcpy.ll
    llvm/test/Transforms/PGOProfile/memop_profile_funclet.ll
    llvm/unittests/ProfileData/CMakeLists.txt

Removed: 
    llvm/unittests/ProfileData/InstrProfDataTest.cpp


################################################################################
diff  --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
index 6d0ffb12294b..a6913527e67f 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -157,8 +157,6 @@ VALUE_PROF_FUNC_PARAM(void *, Data, Type::getInt8PtrTy(Ctx)) INSTR_PROF_COMMA
 #ifndef VALUE_RANGE_PROF
 VALUE_PROF_FUNC_PARAM(uint32_t, CounterIndex, Type::getInt32Ty(Ctx))
 #else /* VALUE_RANGE_PROF */
-/* FIXME: This is to be removed after switching to the new memop value
- * profiling. */
 VALUE_PROF_FUNC_PARAM(uint32_t, CounterIndex, Type::getInt32Ty(Ctx)) \
                       INSTR_PROF_COMMA
 VALUE_PROF_FUNC_PARAM(uint64_t, PreciseRangeStart, Type::getInt64Ty(Ctx)) \
@@ -755,14 +753,9 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 #define INSTR_PROF_VALUE_PROF_FUNC __llvm_profile_instrument_target
 #define INSTR_PROF_VALUE_PROF_FUNC_STR \
         INSTR_PROF_QUOTE(INSTR_PROF_VALUE_PROF_FUNC)
-/* FIXME: This is to be removed after switching to the new memop value
- * profiling. */
 #define INSTR_PROF_VALUE_RANGE_PROF_FUNC __llvm_profile_instrument_range
 #define INSTR_PROF_VALUE_RANGE_PROF_FUNC_STR \
         INSTR_PROF_QUOTE(INSTR_PROF_VALUE_RANGE_PROF_FUNC)
-#define INSTR_PROF_VALUE_PROF_MEMOP_FUNC __llvm_profile_instrument_memop
-#define INSTR_PROF_VALUE_PROF_MEMOP_FUNC_STR                                   \
-  INSTR_PROF_QUOTE(INSTR_PROF_VALUE_PROF_MEMOP_FUNC)
 
 /* InstrProfile per-function control data alignment.  */
 #define INSTR_PROF_DATA_ALIGNMENT 8
@@ -790,121 +783,3 @@ typedef struct InstrProfValueData {
 #endif
 
 #undef COVMAP_V2_OR_V3
-
-#ifdef INSTR_PROF_VALUE_PROF_MEMOP_API
-
-#ifdef __cplusplus
-#define INSTR_PROF_INLINE inline
-#else
-#define INSTR_PROF_INLINE
-#endif
-
-/* The value range buckets (22 buckets) for the memop size value profiling looks
- * like:
- *
- *   [0, 0]
- *   [1, 1]
- *   [2, 2]
- *   [3, 3]
- *   [4, 4]
- *   [5, 5]
- *   [6, 6]
- *   [7, 7]
- *   [8, 8]
- *   [9, 15]
- *   [16, 16]
- *   [17, 31]
- *   [32, 32]
- *   [33, 63]
- *   [64, 64]
- *   [65, 127]
- *   [128, 128]
- *   [129, 255]
- *   [256, 256]
- *   [257, 511]
- *   [512, 512]
- *   [513, UINT64_MAX]
- *
- * Each range has a 'representative value' which is the lower end value of the
- * range and used to store in the runtime profile data records and the VP
- * metadata. For example, it's 2 for [2, 2] and 64 for [65, 127].
- */
-
-/*
- * Clz and Popcount. This code was copied from
- * compiler-rt/lib/fuzzer/{FuzzerBuiltins.h,FuzzerBuiltinsMsvc.h} and
- * llvm/include/llvm/Support/MathExtras.h.
- */
-#if defined(_MSC_VER) && !defined(__clang__)
-
-#include <intrin.h>
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
-int InstProfClzll(unsigned long long X) {
-  unsigned long LeadZeroIdx = 0;
-#if !defined(_M_ARM64) && !defined(_M_X64)
-  // Scan the high 32 bits.
-  if (_BitScanReverse(&LeadZeroIdx, (unsigned long)(X >> 32)))
-    return (int)(63 - (LeadZeroIdx + 32)); // Create a bit offset
-                                                      // from the MSB.
-  // Scan the low 32 bits.
-  if (_BitScanReverse(&LeadZeroIdx, (unsigned long)(X)))
-    return (int)(63 - LeadZeroIdx);
-#else
-  if (_BitScanReverse64(&LeadZeroIdx, X)) return 63 - LeadZeroIdx;
-#endif
-  return 64;
-}
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
-int InstProfPopcountll(unsigned long long X) {
-  // This code originates from https://reviews.llvm.org/rG30626254510f.
-  unsigned long long v = X;
-  v = v - ((v >> 1) & 0x5555555555555555ULL);
-  v = (v & 0x3333333333333333ULL) + ((v >> 2) & 0x3333333333333333ULL);
-  v = (v + (v >> 4)) & 0x0F0F0F0F0F0F0F0FULL;
-  return (int)((unsigned long long)(v * 0x0101010101010101ULL) >> 56);
-}
-
-#else
-
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
-int InstProfClzll(unsigned long long X) { return __builtin_clzll(X); }
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
-int InstProfPopcountll(unsigned long long X) { return __builtin_popcountll(X); }
-
-#endif  /* defined(_MSC_VER) && !defined(__clang__) */
-
-/* Map an (observed) memop size value to the representative value of its range.
- * For example, 5 -> 5, 22 -> 17, 99 -> 65, 256 -> 256, 1001 -> 513. */
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE uint64_t
-InstrProfGetRangeRepValue(uint64_t Value) {
-  if (Value <= 8)
-    // The first ranges are individually tracked. Use the value as is.
-    return Value;
-  else if (Value >= 513)
-    // The last range is mapped to its lowest value.
-    return 513;
-  else if (InstProfPopcountll(Value) == 1)
-    // If it's a power of two, use it as is.
-    return Value;
-  else
-    // Otherwise, take to the previous power of two + 1.
-    return (1 << (64 - InstProfClzll(Value) - 1)) + 1;
-}
-
-/* Return true if the range that an (observed) memop size value belongs to has
- * only a single value in the range.  For example, 0 -> true, 8 -> true, 10 ->
- * false, 64 -> true, 100 -> false, 513 -> false. */
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE unsigned
-InstrProfIsSingleValRange(uint64_t Value) {
-  if (Value <= 8)
-    // The first ranges are individually tracked.
-    return 1;
-  else if (InstProfPopcountll(Value) == 1)
-    // If it's a power of two, there's only one value.
-    return 1;
-  else
-    // Otherwise, there's more than one value in the range.
-    return 0;
-}
-
-#endif /* INSTR_PROF_VALUE_PROF_MEMOP_API */

diff  --git a/compiler-rt/lib/profile/InstrProfilingValue.c b/compiler-rt/lib/profile/InstrProfilingValue.c
index 76e1d3fa11b8..fd53cac3dff3 100644
--- a/compiler-rt/lib/profile/InstrProfilingValue.c
+++ b/compiler-rt/lib/profile/InstrProfilingValue.c
@@ -17,14 +17,13 @@
 
 #define INSTR_PROF_VALUE_PROF_DATA
 #define INSTR_PROF_COMMON_API_IMPL
-#define INSTR_PROF_VALUE_PROF_MEMOP_API
 #include "profile/InstrProfData.inc"
 
 static int hasStaticCounters = 1;
 static int OutOfNodesWarnings = 0;
 static int hasNonDefaultValsPerSite = 0;
 #define INSTR_PROF_MAX_VP_WARNS 10
-#define INSTR_PROF_DEFAULT_NUM_VAL_PER_SITE 24
+#define INSTR_PROF_DEFAULT_NUM_VAL_PER_SITE 16
 #define INSTR_PROF_VNODE_POOL_SIZE 1024
 
 #ifndef _MSC_VER
@@ -251,8 +250,6 @@ __llvm_profile_instrument_target_value(uint64_t TargetValue, void *Data,
  * The range for large values is optional. The default value of INT64_MIN
  * indicates it is not specified.
  */
-/* FIXME: This is to be removed after switching to the new memop value
- * profiling. */
 COMPILER_RT_VISIBILITY void __llvm_profile_instrument_range(
     uint64_t TargetValue, void *Data, uint32_t CounterIndex,
     int64_t PreciseRangeStart, int64_t PreciseRangeLast, int64_t LargeValue) {
@@ -266,18 +263,6 @@ COMPILER_RT_VISIBILITY void __llvm_profile_instrument_range(
   __llvm_profile_instrument_target(TargetValue, Data, CounterIndex);
 }
 
-/*
- * The target values are partitioned into multiple ranges. The range spec is
- * defined in InstrProfData.inc.
- */
-COMPILER_RT_VISIBILITY void
-__llvm_profile_instrument_memop(uint64_t TargetValue, void *Data,
-                                uint32_t CounterIndex) {
-  // Map the target value to the representative value of its range.
-  uint64_t RepValue = InstrProfGetRangeRepValue(TargetValue);
-  __llvm_profile_instrument_target(RepValue, Data, CounterIndex);
-}
-
 /*
  * A wrapper struct that represents value profile runtime data.
  * Like InstrProfRecord class which is used by profiling host tools,

diff  --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 010f1d9432ee..a3359ca90133 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -75,18 +75,10 @@ inline StringRef getInstrProfValueProfFuncName() {
 }
 
 /// Return the name profile runtime entry point to do value range profiling.
-// FIXME: This is to be removed after switching to the new memop value
-// profiling.
 inline StringRef getInstrProfValueRangeProfFuncName() {
   return INSTR_PROF_VALUE_RANGE_PROF_FUNC_STR;
 }
 
-/// Return the name profile runtime entry point to do memop size value
-/// profiling.
-inline StringRef getInstrProfValueProfMemOpFuncName() {
-  return INSTR_PROF_VALUE_PROF_MEMOP_FUNC_STR;
-}
-
 /// Return the name prefix of variables containing instrumented function names.
 inline StringRef getInstrProfNameVarPrefix() { return "__profn_"; }
 

diff  --git a/llvm/include/llvm/ProfileData/InstrProfData.inc b/llvm/include/llvm/ProfileData/InstrProfData.inc
index 06927fb5652b..e56623afad64 100644
--- a/llvm/include/llvm/ProfileData/InstrProfData.inc
+++ b/llvm/include/llvm/ProfileData/InstrProfData.inc
@@ -157,8 +157,6 @@ VALUE_PROF_FUNC_PARAM(void *, Data, Type::getInt8PtrTy(Ctx)) INSTR_PROF_COMMA
 #ifndef VALUE_RANGE_PROF
 VALUE_PROF_FUNC_PARAM(uint32_t, CounterIndex, Type::getInt32Ty(Ctx))
 #else /* VALUE_RANGE_PROF */
-/* FIXME: This is to be removed after switching to the new memop value
- * profiling. */
 VALUE_PROF_FUNC_PARAM(uint32_t, CounterIndex, Type::getInt32Ty(Ctx)) \
                       INSTR_PROF_COMMA
 VALUE_PROF_FUNC_PARAM(uint64_t, PreciseRangeStart, Type::getInt64Ty(Ctx)) \
@@ -756,14 +754,9 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 #define INSTR_PROF_VALUE_PROF_FUNC __llvm_profile_instrument_target
 #define INSTR_PROF_VALUE_PROF_FUNC_STR \
         INSTR_PROF_QUOTE(INSTR_PROF_VALUE_PROF_FUNC)
-/* FIXME: This is to be removed after switching to the new memop value
- * profiling. */
 #define INSTR_PROF_VALUE_RANGE_PROF_FUNC __llvm_profile_instrument_range
 #define INSTR_PROF_VALUE_RANGE_PROF_FUNC_STR \
         INSTR_PROF_QUOTE(INSTR_PROF_VALUE_RANGE_PROF_FUNC)
-#define INSTR_PROF_VALUE_PROF_MEMOP_FUNC __llvm_profile_instrument_memop
-#define INSTR_PROF_VALUE_PROF_MEMOP_FUNC_STR                                   \
-  INSTR_PROF_QUOTE(INSTR_PROF_VALUE_PROF_MEMOP_FUNC)
 
 /* InstrProfile per-function control data alignment.  */
 #define INSTR_PROF_DATA_ALIGNMENT 8
@@ -791,121 +784,3 @@ typedef struct InstrProfValueData {
 #endif
 
 #undef COVMAP_V2_OR_V3
-
-#ifdef INSTR_PROF_VALUE_PROF_MEMOP_API
-
-#ifdef __cplusplus
-#define INSTR_PROF_INLINE inline
-#else
-#define INSTR_PROF_INLINE
-#endif
-
-/* The value range buckets (22 buckets) for the memop size value profiling looks
- * like:
- *
- *   [0, 0]
- *   [1, 1]
- *   [2, 2]
- *   [3, 3]
- *   [4, 4]
- *   [5, 5]
- *   [6, 6]
- *   [7, 7]
- *   [8, 8]
- *   [9, 15]
- *   [16, 16]
- *   [17, 31]
- *   [32, 32]
- *   [33, 63]
- *   [64, 64]
- *   [65, 127]
- *   [128, 128]
- *   [129, 255]
- *   [256, 256]
- *   [257, 511]
- *   [512, 512]
- *   [513, UINT64_MAX]
- *
- * Each range has a 'representative value' which is the lower end value of the
- * range and used to store in the runtime profile data records and the VP
- * metadata. For example, it's 2 for [2, 2] and 64 for [65, 127].
- */
-
-/*
- * Clz and Popcount. This code was copied from
- * compiler-rt/lib/fuzzer/{FuzzerBuiltins.h,FuzzerBuiltinsMsvc.h} and
- * llvm/include/llvm/Support/MathExtras.h.
- */
-#if defined(_MSC_VER) && !defined(__clang__)
-
-#include <intrin.h>
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
-int InstProfClzll(unsigned long long X) {
-  unsigned long LeadZeroIdx = 0;
-#if !defined(_M_ARM64) && !defined(_M_X64)
-  // Scan the high 32 bits.
-  if (_BitScanReverse(&LeadZeroIdx, (unsigned long)(X >> 32)))
-    return (int)(63 - (LeadZeroIdx + 32)); // Create a bit offset
-                                                      // from the MSB.
-  // Scan the low 32 bits.
-  if (_BitScanReverse(&LeadZeroIdx, (unsigned long)(X)))
-    return (int)(63 - LeadZeroIdx);
-#else
-  if (_BitScanReverse64(&LeadZeroIdx, X)) return 63 - LeadZeroIdx;
-#endif
-  return 64;
-}
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
-int InstProfPopcountll(unsigned long long X) {
-  // This code originates from https://reviews.llvm.org/rG30626254510f.
-  unsigned long long v = X;
-  v = v - ((v >> 1) & 0x5555555555555555ULL);
-  v = (v & 0x3333333333333333ULL) + ((v >> 2) & 0x3333333333333333ULL);
-  v = (v + (v >> 4)) & 0x0F0F0F0F0F0F0F0FULL;
-  return (int)((unsigned long long)(v * 0x0101010101010101ULL) >> 56);
-}
-
-#else
-
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
-int InstProfClzll(unsigned long long X) { return __builtin_clzll(X); }
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
-int InstProfPopcountll(unsigned long long X) { return __builtin_popcountll(X); }
-
-#endif  /* defined(_MSC_VER) && !defined(__clang__) */
-
-/* Map an (observed) memop size value to the representative value of its range.
- * For example, 5 -> 5, 22 -> 17, 99 -> 65, 256 -> 256, 1001 -> 513. */
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE uint64_t
-InstrProfGetRangeRepValue(uint64_t Value) {
-  if (Value <= 8)
-    // The first ranges are individually tracked. Use the value as is.
-    return Value;
-  else if (Value >= 513)
-    // The last range is mapped to its lowest value.
-    return 513;
-  else if (InstProfPopcountll(Value) == 1)
-    // If it's a power of two, use it as is.
-    return Value;
-  else
-    // Otherwise, take to the previous power of two + 1.
-    return (1 << (64 - InstProfClzll(Value) - 1)) + 1;
-}
-
-/* Return true if the range that an (observed) memop size value belongs to has
- * only a single value in the range.  For example, 0 -> true, 8 -> true, 10 ->
- * false, 64 -> true, 100 -> false, 513 -> false. */
-INSTR_PROF_VISIBILITY INSTR_PROF_INLINE unsigned
-InstrProfIsSingleValRange(uint64_t Value) {
-  if (Value <= 8)
-    // The first ranges are individually tracked.
-    return 1;
-  else if (InstProfPopcountll(Value) == 1)
-    // If it's a power of two, there's only one value.
-    return 1;
-  else
-    // Otherwise, there's more than one value in the range.
-    return 0;
-}
-
-#endif /* INSTR_PROF_VALUE_PROF_MEMOP_API */

diff  --git a/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h b/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
index a7052f7b6a2b..263d3b629589 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
@@ -68,8 +68,6 @@ class InstrProfiling : public PassInfoMixin<InstrProfiling> {
   // vector of counter load/store pairs to be register promoted.
   std::vector<LoadStorePair> PromotionCandidates;
 
-  // FIXME: These are to be removed after switching to the new memop value
-  // profiling.
   // The start value of precise value profile range for memory intrinsic sizes.
   int64_t MemOPSizeRangeStart;
   // The end value of precise value profile range for memory intrinsic sizes.

diff  --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 9891e3ea9aec..9b429bf37d74 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1111,8 +1111,6 @@ bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken) {
   return true;
 }
 
-// FIXME: This is to be removed after switching to the new memop value
-// profiling.
 // Parse the value profile options.
 void getMemOPSizeRangeFromOption(StringRef MemOPSizeRange, int64_t &RangeStart,
                                  int64_t &RangeLast) {

diff  --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index 0a3519502994..7b03bbfcdfe4 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -57,8 +57,6 @@ using namespace llvm;
 
 #define DEBUG_TYPE "instrprof"
 
-// FIXME: These are to be removed after switching to the new memop value
-// profiling.
 // The start and end values of precise value profile range for memory
 // intrinsic sizes
 cl::opt<std::string> MemOPSizeRange(
@@ -74,12 +72,6 @@ cl::opt<unsigned> MemOPSizeLarge(
              "Value of 0 disables the large value profiling."),
     cl::init(8192));
 
-cl::opt<bool> UseOldMemOpValueProf(
-    "use-old-memop-value-prof",
-    cl::desc("Use the old memop value profiling buckets. This is "
-             "transitional and to be removed after switching. "),
-    cl::init(true));
-
 namespace {
 
 cl::opt<bool> DoHashBasedCounterSplit(
@@ -403,19 +395,6 @@ class PGOCounterPromoter {
   BlockFrequencyInfo *BFI;
 };
 
-enum class ValueProfilingCallType {
-  // Individual values are tracked. Currently used for indiret call target
-  // profiling.
-  Default,
-
-  // The old memop size value profiling. FIXME: To be removed after switching to
-  // the new one.
-  OldMemOp,
-
-  // MemOp: the (new) memop size value profiling with extended buckets.
-  MemOp
-};
-
 } // end anonymous namespace
 
 PreservedAnalyses InstrProfiling::run(Module &M, ModuleAnalysisManager &AM) {
@@ -600,9 +579,9 @@ bool InstrProfiling::run(
   return true;
 }
 
-static FunctionCallee getOrInsertValueProfilingCall(
-    Module &M, const TargetLibraryInfo &TLI,
-    ValueProfilingCallType CallType = ValueProfilingCallType::Default) {
+static FunctionCallee
+getOrInsertValueProfilingCall(Module &M, const TargetLibraryInfo &TLI,
+                              bool IsRange = false) {
   LLVMContext &Ctx = M.getContext();
   auto *ReturnTy = Type::getVoidTy(M.getContext());
 
@@ -610,22 +589,16 @@ static FunctionCallee getOrInsertValueProfilingCall(
   if (auto AK = TLI.getExtAttrForI32Param(false))
     AL = AL.addParamAttribute(M.getContext(), 2, AK);
 
-  if (CallType == ValueProfilingCallType::Default ||
-      CallType == ValueProfilingCallType::MemOp) {
+  if (!IsRange) {
     Type *ParamTypes[] = {
 #define VALUE_PROF_FUNC_PARAM(ParamType, ParamName, ParamLLVMType) ParamLLVMType
 #include "llvm/ProfileData/InstrProfData.inc"
     };
     auto *ValueProfilingCallTy =
         FunctionType::get(ReturnTy, makeArrayRef(ParamTypes), false);
-    StringRef FuncName = CallType == ValueProfilingCallType::Default
-                             ? getInstrProfValueProfFuncName()
-                             : getInstrProfValueProfMemOpFuncName();
-    return M.getOrInsertFunction(FuncName, ValueProfilingCallTy, AL);
+    return M.getOrInsertFunction(getInstrProfValueProfFuncName(),
+                                 ValueProfilingCallTy, AL);
   } else {
-    // FIXME: This code is to be removed after switching to the new memop value
-    // profiling.
-    assert(CallType == ValueProfilingCallType::OldMemOp);
     Type *RangeParamTypes[] = {
 #define VALUE_RANGE_PROF 1
 #define VALUE_PROF_FUNC_PARAM(ParamType, ParamName, ParamLLVMType) ParamLLVMType
@@ -665,8 +638,8 @@ void InstrProfiling::lowerValueProfileInst(InstrProfValueProfileInst *Ind) {
     Index += It->second.NumValueSites[Kind];
 
   IRBuilder<> Builder(Ind);
-  bool IsMemOpSize = (Ind->getValueKind()->getZExtValue() ==
-                      llvm::InstrProfValueKind::IPVK_MemOPSize);
+  bool IsRange = (Ind->getValueKind()->getZExtValue() ==
+                  llvm::InstrProfValueKind::IPVK_MemOPSize);
   CallInst *Call = nullptr;
   auto *TLI = &GetTLI(*Ind->getFunction());
 
@@ -676,19 +649,12 @@ void InstrProfiling::lowerValueProfileInst(InstrProfValueProfileInst *Ind) {
   // WinEHPrepare pass.
   SmallVector<OperandBundleDef, 1> OpBundles;
   Ind->getOperandBundlesAsDefs(OpBundles);
-  if (!IsMemOpSize) {
+  if (!IsRange) {
     Value *Args[3] = {Ind->getTargetValue(),
                       Builder.CreateBitCast(DataVar, Builder.getInt8PtrTy()),
                       Builder.getInt32(Index)};
     Call = Builder.CreateCall(getOrInsertValueProfilingCall(*M, *TLI), Args,
                               OpBundles);
-  } else if (!UseOldMemOpValueProf) {
-    Value *Args[3] = {Ind->getTargetValue(),
-                      Builder.CreateBitCast(DataVar, Builder.getInt8PtrTy()),
-                      Builder.getInt32(Index)};
-    Call = Builder.CreateCall(
-        getOrInsertValueProfilingCall(*M, *TLI, ValueProfilingCallType::MemOp),
-        Args, OpBundles);
   } else {
     Value *Args[6] = {
         Ind->getTargetValue(),
@@ -697,8 +663,7 @@ void InstrProfiling::lowerValueProfileInst(InstrProfValueProfileInst *Ind) {
         Builder.getInt64(MemOPSizeRangeStart),
         Builder.getInt64(MemOPSizeRangeLast),
         Builder.getInt64(MemOPSizeLarge == 0 ? INT64_MIN : MemOPSizeLarge)};
-    Call = Builder.CreateCall(getOrInsertValueProfilingCall(
-                                  *M, *TLI, ValueProfilingCallType::OldMemOp),
+    Call = Builder.CreateCall(getOrInsertValueProfilingCall(*M, *TLI, true),
                               Args, OpBundles);
   }
   if (auto AK = TLI->getExtAttrForI32Param(false))

diff  --git a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
index 43a1434ae2d3..2b7b859891dc 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
@@ -38,8 +38,6 @@
 #include "llvm/Pass.h"
 #include "llvm/PassRegistry.h"
 #include "llvm/ProfileData/InstrProf.h"
-#define INSTR_PROF_VALUE_PROF_MEMOP_API
-#include "llvm/ProfileData/InstrProfData.inc"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
@@ -91,25 +89,17 @@ static cl::opt<bool>
                     cl::desc("Scale the memop size counts using the basic "
                              " block count value"));
 
-// FIXME: These are to be removed after switching to the new memop value
-// profiling.
 // This option sets the rangge of precise profile memop sizes.
 extern cl::opt<std::string> MemOPSizeRange;
 
 // This option sets the value that groups large memop sizes
 extern cl::opt<unsigned> MemOPSizeLarge;
 
-extern cl::opt<bool> UseOldMemOpValueProf;
-
 cl::opt<bool>
     MemOPOptMemcmpBcmp("pgo-memop-optimize-memcmp-bcmp", cl::init(true),
                        cl::Hidden,
                        cl::desc("Size-specialize memcmp and bcmp calls"));
 
-static cl::opt<unsigned>
-    MemOpMaxOptSize("memop-value-prof-max-opt-size", cl::Hidden, cl::init(128),
-                    cl::desc("Optimize the memop size <= this value"));
-
 namespace {
 class PGOMemOPSizeOptLegacyPass : public FunctionPass {
 public:
@@ -279,8 +269,6 @@ class MemOPSizeOpt : public InstVisitor<MemOPSizeOpt> {
   TargetLibraryInfo &TLI;
   bool Changed;
   std::vector<MemOp> WorkList;
-  // FIXME: These are to be removed after switching to the new memop value
-  // profiling.
   // Start of the previse range.
   int64_t PreciseRangeStart;
   // Last value of the previse range.
@@ -289,8 +277,6 @@ class MemOPSizeOpt : public InstVisitor<MemOPSizeOpt> {
   std::unique_ptr<InstrProfValueData[]> ValueDataArray;
   bool perform(MemOp MO);
 
-  // FIXME: This is to be removed after switching to the new memop value
-  // profiling.
   // This kind shows which group the value falls in. For PreciseValue, we have
   // the profile count for that value. LargeGroup groups the values that are in
   // range [LargeValue, +inf). NonLargeGroup groups the rest of values.
@@ -379,11 +365,8 @@ bool MemOPSizeOpt::perform(MemOp MO) {
     if (MemOPScaleCount)
       C = getScaledCount(C, ActualCount, SavedTotalCount);
 
-    if (UseOldMemOpValueProf) {
-      // Only care precise value here.
-      if (getMemOPSizeKind(V) != PreciseValue)
-        continue;
-    } else if (!InstrProfIsSingleValRange(V) || V > MemOpMaxOptSize)
+    // Only care precise value here.
+    if (getMemOPSizeKind(V) != PreciseValue)
       continue;
 
     // ValueCounts are sorted on the count. Break at the first un-profitable

diff  --git a/llvm/test/Transforms/PGOProfile/memcpy.ll b/llvm/test/Transforms/PGOProfile/memcpy.ll
index e00e1d350871..6047c95e7c08 100644
--- a/llvm/test/Transforms/PGOProfile/memcpy.ll
+++ b/llvm/test/Transforms/PGOProfile/memcpy.ll
@@ -1,7 +1,5 @@
-; RUN: opt < %s -pgo-instr-gen -instrprof -use-old-memop-value-prof=true -S | FileCheck %s --check-prefix=OLDMEMOPVP
-; RUN: opt < %s -pgo-instr-gen -instrprof -use-old-memop-value-prof=false -S | FileCheck %s --check-prefix=NEWMEMOPVP
-; RUN: opt <%s -passes=pgo-instr-gen,instrprof -use-old-memop-value-prof=true -S | FileCheck %s --check-prefix=OLDMEMOPVP
-; RUN: opt <%s -passes=pgo-instr-gen,instrprof -use-old-memop-value-prof=false -S | FileCheck %s --check-prefix=NEWMEMOPVP
+; RUN: opt < %s -pgo-instr-gen -instrprof -S | FileCheck %s
+; RUN: opt <%s -passes=pgo-instr-gen,instrprof -S | FileCheck %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -25,8 +23,7 @@ for.cond1:
 
 for.body3:
   %conv = sext i32 %add to i64
-; OLDMEMOPVP: call void @__llvm_profile_instrument_range(i64 %conv, i8* bitcast ({ i64, i64, i64*, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 0, i64 0, i64 8, i64 8192)
-; NEWMEMOPVP: call void @__llvm_profile_instrument_memop(i64 %conv, i8* bitcast ({ i64, i64, i64*, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 0)
+; CHECK: call void @__llvm_profile_instrument_range(i64 %conv, i8* bitcast ({ i64, i64, i64*, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 0, i64 0, i64 8, i64 8192)
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %conv, i1 false)
   %inc = add nsw i32 %j.0, 1
   br label %for.cond1

diff  --git a/llvm/test/Transforms/PGOProfile/memop_profile_funclet.ll b/llvm/test/Transforms/PGOProfile/memop_profile_funclet.ll
index 43c85ed25baa..b79431e3128e 100644
--- a/llvm/test/Transforms/PGOProfile/memop_profile_funclet.ll
+++ b/llvm/test/Transforms/PGOProfile/memop_profile_funclet.ll
@@ -1,10 +1,8 @@
 ; RUN: opt < %s -pgo-instr-gen -S | FileCheck %s --check-prefix=GEN
-; RUN: opt < %s -pgo-instr-gen -instrprof -use-old-memop-value-prof=true -S | FileCheck %s --check-prefixes=LOWER,LOWEROLDMEMOPVP
-; RUN: opt < %s -pgo-instr-gen -instrprof -use-old-memop-value-prof=false -S | FileCheck %s --check-prefixes=LOWER,LOWERNEWMEMOPVP
+; RUN: opt < %s -pgo-instr-gen -instrprof -S | FileCheck %s --check-prefix=LOWER
 
 ; RUN: opt < %s -passes=pgo-instr-gen -S | FileCheck %s --check-prefix=GEN
-; RUN: opt < %s -passes=pgo-instr-gen,instrprof -use-old-memop-value-prof=true -S | FileCheck %s --check-prefixes=LOWER,LOWEROLDMEMOPVP
-; RUN: opt < %s -passes=pgo-instr-gen,instrprof -use-old-memop-value-prof=false -S | FileCheck %s --check-prefixes=LOWER,LOWERNEWMEMOPVP
+; RUN: opt < %s -passes=pgo-instr-gen,instrprof -S | FileCheck %s --check-prefix=LOWER
 
 ; This test is to verify that PGO runtime library calls get created with the
 ; appropriate operand bundle funclet information when a memory intrinsic
@@ -65,8 +63,7 @@ try.cont:                                         ; preds = %entry
 ; GEN-SAME: [ "funclet"(token %tmp1) ]
 
 ; LOWER: catch:
-; LOWEROLDMEMOPVP: call void @__llvm_profile_instrument_range(
-; LOWERNEWMEMOPVP: call void @__llvm_profile_instrument_memop(
+; LOWER: call void @__llvm_profile_instrument_range(
 ; LOWER-SAME: [ "funclet"(token %tmp1) ]
 
 declare dso_local void @"?may_throw@@YAXH at Z"(i32)

diff  --git a/llvm/unittests/ProfileData/CMakeLists.txt b/llvm/unittests/ProfileData/CMakeLists.txt
index 00a0079e675a..366ed5482bf2 100644
--- a/llvm/unittests/ProfileData/CMakeLists.txt
+++ b/llvm/unittests/ProfileData/CMakeLists.txt
@@ -7,7 +7,6 @@ set(LLVM_LINK_COMPONENTS
 
 add_llvm_unittest(ProfileDataTests
   CoverageMappingTest.cpp
-  InstrProfDataTest.cpp
   InstrProfTest.cpp
   SampleProfTest.cpp
   )

diff  --git a/llvm/unittests/ProfileData/InstrProfDataTest.cpp b/llvm/unittests/ProfileData/InstrProfDataTest.cpp
deleted file mode 100644
index af1a3de0657c..000000000000
--- a/llvm/unittests/ProfileData/InstrProfDataTest.cpp
+++ /dev/null
@@ -1,68 +0,0 @@
-//===- unittest/ProfileData/InstProfDataTest.cpp ----------------------------=//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "gtest/gtest.h"
-
-#include <cstdint>
-
-#define INSTR_PROF_VALUE_PROF_MEMOP_API
-#include "llvm/ProfileData/InstrProfData.inc"
-
-namespace {
-
-TEST(InstrProfDataTest, MapValueToRangeRepValue) {
-  EXPECT_EQ(0ULL, InstrProfGetRangeRepValue(0));
-  EXPECT_EQ(1ULL, InstrProfGetRangeRepValue(1));
-  EXPECT_EQ(2ULL, InstrProfGetRangeRepValue(2));
-  EXPECT_EQ(3ULL, InstrProfGetRangeRepValue(3));
-  EXPECT_EQ(4ULL, InstrProfGetRangeRepValue(4));
-  EXPECT_EQ(5ULL, InstrProfGetRangeRepValue(5));
-  EXPECT_EQ(6ULL, InstrProfGetRangeRepValue(6));
-  EXPECT_EQ(7ULL, InstrProfGetRangeRepValue(7));
-  EXPECT_EQ(8ULL, InstrProfGetRangeRepValue(8));
-  EXPECT_EQ(9ULL, InstrProfGetRangeRepValue(9));
-  EXPECT_EQ(16ULL, InstrProfGetRangeRepValue(16));
-  EXPECT_EQ(17ULL, InstrProfGetRangeRepValue(30));
-  EXPECT_EQ(32ULL, InstrProfGetRangeRepValue(32));
-  EXPECT_EQ(33ULL, InstrProfGetRangeRepValue(54));
-  EXPECT_EQ(64ULL, InstrProfGetRangeRepValue(64));
-  EXPECT_EQ(65ULL, InstrProfGetRangeRepValue(127));
-  EXPECT_EQ(128ULL, InstrProfGetRangeRepValue(128));
-  EXPECT_EQ(129ULL, InstrProfGetRangeRepValue(200));
-  EXPECT_EQ(256ULL, InstrProfGetRangeRepValue(256));
-  EXPECT_EQ(257ULL, InstrProfGetRangeRepValue(397));
-  EXPECT_EQ(512ULL, InstrProfGetRangeRepValue(512));
-  EXPECT_EQ(513ULL, InstrProfGetRangeRepValue(2832048023));
-}
-
-TEST(InstrProfDataTest, IsInOneValueRange) {
-  EXPECT_EQ(true, InstrProfIsSingleValRange(0));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(1));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(2));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(3));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(4));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(5));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(6));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(7));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(8));
-  EXPECT_EQ(false, InstrProfIsSingleValRange(9));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(16));
-  EXPECT_EQ(false, InstrProfIsSingleValRange(30));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(32));
-  EXPECT_EQ(false, InstrProfIsSingleValRange(54));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(64));
-  EXPECT_EQ(false, InstrProfIsSingleValRange(127));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(128));
-  EXPECT_EQ(false, InstrProfIsSingleValRange(200));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(256));
-  EXPECT_EQ(false, InstrProfIsSingleValRange(397));
-  EXPECT_EQ(true, InstrProfIsSingleValRange(512));
-  EXPECT_EQ(false, InstrProfIsSingleValRange(2832048023344));
-}
-
-} // end anonymous namespace


        


More information about the llvm-commits mailing list