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

[hjyamauchi]

Summary

Extend the memop value profile buckets to be more flexible (could accommodate a
mix of individual values and ranges) and to cover more value ranges (from 11 to
22 buckets).

Disabled behind a flag (to be enabled separately) and the existing code to be
removed later.

[davidxl]

the approach looks sound.

compiler-rt/include/profile/InstrProfData.inc

168

add FIXME

170

unwanted format change?

810

should 5,6, and 7 be merged into one range?

compiler-rt/lib/profile/InstrProfilingValue.c

254

add FIXME

llvm/include/llvm/ProfileData/InstrProf.h

78

FIXME

[hjyamauchi]

Address comments.

compiler-rt/include/profile/InstrProfData.inc

810

This is as intended because some mem ops could have separate code paths for 5, 6, and 7 (at least our memcmp does in fact) and because the old buckets were 0-9 individually. I'd like the new buckets to be flexible to accommodate those and to supercede the old buckets.

[davidxl]

compiler-rt/include/profile/InstrProfData.inc

836

The name of the interface is too long.

InstrProfGetRangeRepValue would be good enough.

855

InstrProfIsSingleValRange(uint64_t RepValue)

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

620

this branch can be merged with the above -- though it is to be deprecated.

[hjyamauchi]

Address comments.

[hjyamauchi]

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

620

They can be merged into something like:

    Type *ParamTypes[] = {
#define VALUE_PROF_FUNC_PARAM(ParamType, ParamName, ParamLLVMType) ParamLLVMType
#include "llvm/ProfileData/InstrProfData.inc"
    };

    Type *RangeParamTypes[] = {
#define VALUE_RANGE_PROF 1
#define VALUE_PROF_FUNC_PARAM(ParamType, ParamName, ParamLLVMType) ParamLLVMType
#include "llvm/ProfileData/InstrProfData.inc"
#undef VALUE_RANGE_PROF
    };

    auto *ValueProfilingCallTy =
        CallType != ValueProfilingCallType::OldMemOp
	FunctionType::get(ReturnTy, makeArrayRef(ParamTypes), false) :
        FunctionType::get(ReturnTy, makeArrayRef(RangeParamTypes), false);

    StringRef FuncName = CallType != ValueProfilingCallType::MemOp
                             ? getInstrProfValueProfFuncName()
                             : getInstrProfValueProfMemOpFuncName();

    return M.getOrInsertFunction(FuncName, ValueProfilingCallTy, AL);

which would be a fewer lines shorter and fine.

But I like the current code in that it already makes it more clear-cut what the new code would look like and what will be removed in a future patch that removes the deprecated code (the if statement along with the else block will be simply removed.)

[davidxl]

lgtm

[hjyamauchi]

Reverted due to build failure as https://reviews.llvm.org/rG9878996c7089355afe3e2e23ceb093940345e8b0

[hjyamauchi]

Build error

C:\PROGRA~2\MIB055~1\2017\PROFES~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -DSTDC_CONSTANT_MACROS -DSTDC_FORMAT_MACROS -DSTDC_LIMIT_MACROS -Ilib\Transforms\Instrumentation -IC:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Transforms\Instrumentation -Iinclude -IC:\b\slave\sanitizer-windows\llvm-project\llvm\include /DWIN32 /D_WINDOWS /Zc:inline /Zi /Zc:strictStrings /Oi /Zc:rvalueCast /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2 /EHs-c- /GR- -UNDEBUG /showIncludes /Folib\Transforms\Instrumentation\CMakeFiles\LLVMInstrumentation.dir\PGOMemOPSizeOpt.cpp.obj /Fdlib\Transforms\Instrumentation\CMakeFiles\LLVMInstrumentation.dir\ /FS -c C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Transforms\Instrumentation\PGOMemOPSizeOpt.cpp
C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/ProfileData/InstrProfData.inc(843): error C3861: 'builtin_popcountll': identifier not found
C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/ProfileData/InstrProfData.inc(848): error C3861: 'builtin_clzll': identifier not found
C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/ProfileData/InstrProfData.inc(859): error C3861: 'builtin_popcountll': identifier not found

[hjyamauchi]

Update to fix MSVC build errors.

[hjyamauchi]

Here's the raw diff for the last update

diff --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
index f4cb2524ad7..8c6e6cffe95 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -830,6 +830,47 @@ typedef struct InstrProfValueData {
  * 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}.
+ */
+#if defined(_MSC_VER) && !defined(__clang__)
+
+#include <intrin.h>
+INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
+uint32_t InstProfClzll(uint64_t X) {
+  unsigned long LeadZeroIdx = 0;
+#if !defined(_M_ARM) && !defined(_M_X64)
+  // Scan the high 32 bits.
+  if (_BitScanReverse(&LeadZeroIdx, static_cast<unsigned long>(X >> 32)))
+    return static_cast<int>(63 - (LeadZeroIdx + 32)); // Create a bit offset
+                                                      // from the MSB.
+  // Scan the low 32 bits.
+  if (_BitScanReverse(&LeadZeroIdx, static_cast<unsigned long>(X)))
+    return static_cast<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) {
+#if !defined(_M_ARM) && !defined(_M_X64)
+  return __popcnt(X) + __popcnt(X >> 32);
+#else
+  return __popcnt64(X);
+#endif
+}
+
+#else
+
+INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
+uint32_t 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
@@ -840,12 +881,12 @@ InstrProfGetRangeRepValue(uint64_t Value) {
   else if (Value >= 513)
     // The last range is mapped to its lowest value.
     return 513;
-  else if (__builtin_popcountll(Value) == 1)
+  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 - __builtin_clzll(Value) - 1)) + 1;
+    return (1 << (64 - InstProfClzll(Value) - 1)) + 1;
 }
 
 /* Return true if the range that an (observed) memop size value belongs to has
@@ -856,7 +897,7 @@ InstrProfIsSingleValRange(uint64_t Value) {
   if (Value <= 8)
     // The first ranges are individually tracked.
     return 1;
-  else if (__builtin_popcountll(Value) == 1)
+  else if (InstProfPopcountll(Value) == 1)
     // If it's a power of two, there's only one value.
     return 1;
   else
diff --git a/llvm/include/llvm/ProfileData/InstrProfData.inc b/llvm/include/llvm/ProfileData/InstrProfData.inc
index f4cb2524ad7..8c6e6cffe95 100644
--- a/llvm/include/llvm/ProfileData/InstrProfData.inc
+++ b/llvm/include/llvm/ProfileData/InstrProfData.inc
@@ -830,6 +830,47 @@ typedef struct InstrProfValueData {
  * 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}.
+ */
+#if defined(_MSC_VER) && !defined(__clang__)
+
+#include <intrin.h>
+INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
+uint32_t InstProfClzll(uint64_t X) {
+  unsigned long LeadZeroIdx = 0;
+#if !defined(_M_ARM) && !defined(_M_X64)
+  // Scan the high 32 bits.
+  if (_BitScanReverse(&LeadZeroIdx, static_cast<unsigned long>(X >> 32)))
+    return static_cast<int>(63 - (LeadZeroIdx + 32)); // Create a bit offset
+                                                      // from the MSB.
+  // Scan the low 32 bits.
+  if (_BitScanReverse(&LeadZeroIdx, static_cast<unsigned long>(X)))
+    return static_cast<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) {
+#if !defined(_M_ARM) && !defined(_M_X64)
+  return __popcnt(X) + __popcnt(X >> 32);
+#else
+  return __popcnt64(X);
+#endif
+}
+
+#else
+
+INSTR_PROF_VISIBILITY INSTR_PROF_INLINE
+uint32_t 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
@@ -840,12 +881,12 @@ InstrProfGetRangeRepValue(uint64_t Value) {
   else if (Value >= 513)
     // The last range is mapped to its lowest value.
     return 513;
-  else if (__builtin_popcountll(Value) == 1)
+  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 - __builtin_clzll(Value) - 1)) + 1;
+    return (1 << (64 - InstProfClzll(Value) - 1)) + 1;
 }
 
 /* Return true if the range that an (observed) memop size value belongs to has
@@ -856,7 +897,7 @@ InstrProfIsSingleValRange(uint64_t Value) {
   if (Value <= 8)
     // The first ranges are individually tracked.
     return 1;
-  else if (__builtin_popcountll(Value) == 1)
+  else if (InstProfPopcountll(Value) == 1)
     // If it's a power of two, there's only one value.
     return 1;
   else

[hjyamauchi]

PTAL.

[davidxl]

Added Reid to help review the MS compiler relevant part: https://reviews.llvm.org/D81682/new

[hjyamauchi]

Further refined the MSVC related code.

Based on https://godbolt.org/z/BxlSWX,

• popcnt and popcnt64 aren't available on arm/arm64 and not on all x86/x86-64 CPUs. So, go back to bit twiddling.
• _BitScanReverse64 is available on arm64 and x86-64 only.
• Removed static_cast as this is included in a .c file.

[hjyamauchi]

The relevant MSVC related code now looks like

/*
 * 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__) */

[davidxl]

compiler-rt/include/profile/InstrProfData.inc

839

There is __popcnt etc. Can they be used?

https://docs.microsoft.com/en-us/cpp/intrinsics/popcnt16-popcnt-popcnt64?view=vs-2019

842

Since these helpers are only used by runtime on target, not by the host compiler, they should be moved to InstrProfilingUtil.c instead as InstrProfData.Inc is shared by runtime and compiler.

compiler-rt/lib/profile/InstrProfilingValue.c

274

Ideally, this function should be inline expanded by the compiler at instrumentation time -- but that can be done separately.

[hjyamauchi]

compiler-rt/include/profile/InstrProfData.inc

839

Unfortunately, no. I already tried this. (note "popcnt and popcnt64 aren't available on arm/arm64 and not on all x86/x86-64 CPUs" in the older update message.)

I can confirm it on https://godbolt.org/z/bn5PEy

842

InstrProfIsSingleValRange (hence InstProfPopcountll) is used by the compiler in lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp.

compiler-rt/lib/profile/InstrProfilingValue.c

274

True.

[davidxl]

lgtm

[hjyamauchi]

Committed as https://reviews.llvm.org/rG4a539faf74b9

[dmajor]

The re-landing of this patch in 4a539faf74b9b4c25ee3b880e4007564bd5139b0 causes our PGO build of clang to have thousands of

LLVM Profile Warning: Unable to track new values: Running out of static counters.  Consider using option -mllvm -vp-counters-per-site=<n> to allocate more value profile counters at compile time.

Is that expected? Should the cmake set a higher value of -vp-counters-per-site in self-hosted PGO builds?

[MaskRay]

Chatted with @yamauchi in another channel. Reverted in 27650ec5541cd604a5027ad63895e0badfd35efe

It caused __llvm_profile_instrument_range related crash in PGO-instrumented clang (on many source files):

  1│ Dump of assembler code for function __llvm_profile_instrument_target:
  2│    0x000055555dba0820 <+0>:     push   %rbp
  3│    0x000055555dba0821 <+1>:     mov    %rsp,%rbp
  4│    0x000055555dba0824 <+4>:     push   %r15
  5│    0x000055555dba0826 <+6>:     push   %r14
  6│    0x000055555dba0828 <+8>:     push   %r12
  7│    0x000055555dba082a <+10>:    push   %rbx
  8│    0x000055555dba082b <+11>:    test   %rsi,%rsi
  9│    0x000055555dba082e <+14>:    je     0x55555dba098c <__llvm_profile_instrument_target+364>
 10│    0x000055555dba0834 <+20>:    mov    %edx,%r14d
 11│    0x000055555dba0837 <+23>:    mov    %rsi,%rbx
 12│    0x000055555dba083a <+26>:    mov    %rdi,%r15
 13│    0x000055555dba083d <+29>:    mov    0x20(%rsi),%r12
 14│    0x000055555dba0841 <+33>:    test   %r12,%r12
 15│    0x000055555dba0844 <+36>:    je     0x55555dba08bd <__llvm_profile_instrument_target+157>
 16│    0x000055555dba0846 <+38>:    mov    %r14d,%r14d
 17│    0x000055555dba0849 <+41>:    mov    (%r12,%r14,8),%rsi
 18│    0x000055555dba084d <+45>:    test   %rsi,%rsi
 19│    0x000055555dba0850 <+48>:    je     0x55555dba091a <__llvm_profile_instrument_target+250>
 20│    0x000055555dba0856 <+54>:    mov    $0xffffffffffffffff,%rdx
 21│    0x000055555dba085d <+61>:    xor    %ecx,%ecx
 22│    0x000055555dba085f <+63>:    xor    %eax,%eax
 23│    0x000055555dba0861 <+65>:    nopw   %cs:0x0(%rax,%rax,1)
 24│    0x000055555dba086b <+75>:    nopl   0x0(%rax,%rax,1)
 25│    0x000055555dba0870 <+80>:    mov    %rsi,%rbx
 26│    0x000055555dba0873 <+83>:    mov    0x8(%rsi),%rsi    ####### %rsi=-1 on this line
 27│    0x000055555dba0877 <+87>:    cmp    %r15,(%rbx)
 28│    0x000055555dba087a <+90>:    je     0x55555dba0976 <__llvm_profile_instrument_target+342>
 29│    0x000055555dba0880 <+96>:    cmp    %rdx,%rsi
 30│    0x000055555dba0883 <+99>:    cmovb  %rbx,%rax
 31│    0x000055555dba0887 <+103>:   cmovb  %rsi,%rdx
 32│    0x000055555dba088b <+107>:   add    $0x1,%cl
 33│    0x000055555dba088e <+110>:   mov    0x10(%rbx),%rsi
 34│    0x000055555dba0892 <+114>:   test   %rsi,%rsi
 35│    0x000055555dba0895 <+117>:   jne    0x55555dba0870 <__llvm_profile_instrument_target+80>
 36│    0x000055555dba0897 <+119>:   movzbl %cl,%ecx
 37│    0x000055555dba089a <+122>:   cmp    %ecx,0x6cffac(%rip)        # 0x55555e27084c <VPMaxNumValsPerSite>
** Dump of assembler code for function __llvm_profile_instrument_target: (55555dba0820 - 55555dba0a37) **                                                                                                                               
(gdb) i r rsi
rsi            0xffffffffffffffff  -1
(gdb)

(gdb) bt
#0  0x000055555dba0873 in __llvm_profile_instrument_target ()
#1  0x000055555d88de3f in llvm::APInt::udiv(llvm::APInt const&) const ()
#2  0x000055555d2e203d in getRangeForAffineARHelper(llvm::APInt, llvm::ConstantRange const&, llvm::APInt const&, unsigned int, bool) ()
#3  0x000055555d2e1585 in llvm::ScalarEvolution::getRangeForAffineAR(llvm::SCEV const*, llvm::SCEV const*, llvm::SCEV const*, unsigned int) ()
#4  0x000055555d2dfc38 in llvm::ScalarEvolution::getRangeRef(llvm::SCEV const*, llvm::ScalarEvolution::RangeSignHint) ()

[hjyamauchi]

In D81682#2168028, @dmajor wrote:

The re-landing of this patch in 4a539faf74b9b4c25ee3b880e4007564bd5139b0 causes our PGO build of clang to have thousands of

LLVM Profile Warning: Unable to track new values: Running out of static counters.  Consider using option -mllvm -vp-counters-per-site=<n> to allocate more value profile counters at compile time.

Is that expected? Should the cmake set a higher value of -vp-counters-per-site in self-hosted PGO builds?

This has been reverted for now. There was a related issue.

[dmajor]

In D81682#2168134, @yamauchi wrote:

In D81682#2168028, @dmajor wrote:

The re-landing of this patch in 4a539faf74b9b4c25ee3b880e4007564bd5139b0 causes our PGO build of clang to have thousands of

LLVM Profile Warning: Unable to track new values: Running out of static counters.  Consider using option -mllvm -vp-counters-per-site=<n> to allocate more value profile counters at compile time.

Is that expected? Should the cmake set a higher value of -vp-counters-per-site in self-hosted PGO builds?

This has been reverted for now. There was a related issue.

Our build bots still show lots of this warning, is that expected?

[hjyamauchi]

In D81682#2328239, @dmajor wrote:

In D81682#2168134, @yamauchi wrote:

In D81682#2168028, @dmajor wrote:

The re-landing of this patch in 4a539faf74b9b4c25ee3b880e4007564bd5139b0 causes our PGO build of clang to have thousands of

LLVM Profile Warning: Unable to track new values: Running out of static counters.  Consider using option -mllvm -vp-counters-per-site=<n> to allocate more value profile counters at compile time.

Is that expected? Should the cmake set a higher value of -vp-counters-per-site in self-hosted PGO builds?

This has been reverted for now. There was a related issue.

Our build bots still show lots of this warning, is that expected?

It may actually be that -vp-counters-per-site needs adjusting. How do you reproduce?

[dmajor]

It may actually be that -vp-counters-per-site needs adjusting. How do you reproduce?

This is with clang trying to PGO itself. Build with -DLLVM_BUILD_INSTRUMENTED=IR -DLLVM_BUILD_RUNTIME=No and use the result to build the LLVM tree again.

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318445929&repo=ash&lineNumber=17322

[hjyamauchi]

This is with clang trying to PGO itself. Build with -DLLVM_BUILD_INSTRUMENTED=IR -DLLVM_BUILD_RUNTIME=No and use the result to build the LLVM tree again.

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318445929&repo=ash&lineNumber=17322

It seems that I can't reproduce it. Here's my script. Can you reproduce with it or modify it so it reproduces?

# cd into the root directory "llvm-project"
LLVMROOT=`pwd`

rm -rf $LLVMROOT/clang-bootstrap
mkdir $LLVMROOT/clang-bootstrap
STAGE1=$LLVMROOT/clang-bootstrap/stage1
STAGE2_PROF_GEN=$LLVMROOT/clang-bootstrap/stage2-prof-gen
STAGE2_TRAIN=$LLVMROOT/clang-bootstrap/stage2-train

CMAKE="cmake -G Ninja $LLVMROOT/llvm -DLLVM_ENABLE_PROJECTS=clang;compiler-rt -DLLVM_TARGETS_TO_BUILD=X86 -DCMAKE_BUILD_TYPE=Release -DLLVM_PARALLEL_LINK_JOBS=6"

mkdir $STAGE1
cd $STAGE1
$CMAKE -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ \
  -DCMAKE_INSTALL_PREFIX=$STAGE1/install
ninja install -v

mkdir -p $STAGE2_PROF_GEN
cd $STAGE2_PROF_GEN
$CMAKE -DCMAKE_C_COMPILER=$STAGE1/install/bin/clang \
  -DCMAKE_CXX_COMPILER=$STAGE1/install/bin/clang++ \
  -DLLVM_BUILD_INSTRUMENTED=IR \
  -DLLVM_BUILD_RUNTIME=No \
  -DCMAKE_INSTALL_PREFIX=$STAGE2_PROF_GEN/install
ninja install -v

mkdir $STAGE2_TRAIN
cd $STAGE2_TRAIN
$CMAKE -DCMAKE_C_COMPILER=$STAGE2_PROF_GEN/install/bin/clang \
  -DCMAKE_CXX_COMPILER=$STAGE2_PROF_GEN/install/bin/clang++ \
  -DCMAKE_INSTALL_PREFIX=$STAGE2_TRAIN/install
ninja -v

[dmajor]

Just a quick status update, I don't reproduce the issue with your script. I'm attempting to adjust the script to match our CI, but I'm distracted by other work.

[dmajor]

In D81682#2331245, @yamauchi wrote:

It seems that I can't reproduce it. Here's my script. Can you reproduce with it or modify it so it reproduces?

I'm sorry this took so long, I finally narrowed down the minimal required change. The second stage should have -DLLVM_LINK_LLVM_DYLIB=ON.

[hjyamauchi]

https://reviews.llvm.org/D92669 should fix the issue.

compiler-rt/include/profile/InstrProfData.inc

 #define INSTR_PROF_COMMA ,
 #endif
 VALUE_PROF_FUNC_PARAM(uint64_t, TargetValue, Type::getInt64Ty(Ctx)) \
                       INSTR_PROF_COMMA
 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)) \
                       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))
 #endif /*VALUE_RANGE_PROF */
 #undef VALUE_PROF_FUNC_PARAM
 #undef INSTR_PROF_COMMA
 /* VALUE_PROF_FUNC_PARAM end */
 
 /* VALUE_PROF_KIND start */
 #ifndef VALUE_PROF_KIND
 #define VALUE_PROF_KIND(Enumerator, Value, Descr)
 #else
 #define INSTR_PROF_DATA_DEFINED
#define INSTR_PROF_SECT_START(Sect) \
         INSTR_PROF_CONCAT(__start_,Sect)
 #define INSTR_PROF_SECT_STOP(Sect) \
         INSTR_PROF_CONCAT(__stop_,Sect)
 
 /* Value Profiling API linkage name.  */
 #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
 
 /* The data structure that represents a tracked value by the
  * value profiler.
  */
 typedef struct InstrProfValueData {
   /* Profiled value. */
   uint64_t Value;
   /* Number of times the value appears in the training run. */
   uint64_t Count;
 } InstrProfValueData;
 
 #endif /* INSTR_PROF_DATA_INC */
 
 #ifndef INSTR_ORDER_FILE_INC
 /* The maximal # of functions: 128*1024 (the buffer size will be 128*4 KB). */
 #define INSTR_ORDER_FILE_BUFFER_SIZE 131072
 #define INSTR_ORDER_FILE_BUFFER_BITS 17
 #define INSTR_ORDER_FILE_BUFFER_MASK 0x1ffff
 #endif /* INSTR_ORDER_FILE_INC */
 #else
 #undef INSTR_PROF_DATA_DEFINED
 #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].
+ */
+
+/* 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 (__builtin_popcountll(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 - __builtin_clzll(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 (__builtin_popcountll(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 */

compiler-rt/lib/profile/InstrProfilingValue.c

 #include <string.h>
 
 #include "InstrProfiling.h"
 #include "InstrProfilingInternal.h"
 #include "InstrProfilingUtil.h"
 
 #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 16
+#define INSTR_PROF_DEFAULT_NUM_VAL_PER_SITE 24
 #define INSTR_PROF_VNODE_POOL_SIZE 1024
 
 #ifndef _MSC_VER
 /* A shared static pool in addition to the vnodes statically
  * allocated by the compiler.  */
 COMPILER_RT_VISIBILITY ValueProfNode
     lprofValueProfNodes[INSTR_PROF_VNODE_POOL_SIZE] COMPILER_RT_SECTION(
        COMPILER_RT_SEG INSTR_PROF_VNODES_SECT_NAME);

  * to one region -- all values here should be mapped to one value of
  * "PreciseRangeLast + 1".
  * 2. [PreciseRangeStart, PreciseRangeLast]
  * 3. Large values: [LargeValue, +inf) maps to one value of LargeValue.
  *
  * 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) {
 
   if (LargeValue != INT64_MIN && (int64_t)TargetValue >= LargeValue)
     TargetValue = LargeValue;
   else if ((int64_t)TargetValue < PreciseRangeStart ||
            (int64_t)TargetValue > PreciseRangeLast)
     TargetValue = PreciseRangeLast + 1;
 
   __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,
  * ValueProfRuntimeRecord also implements the abstract intefaces defined in
  * ValueProfRecordClosure so that the runtime data can be serialized using
  * shared C implementation.
  */
 typedef struct ValueProfRuntimeRecord {
   const __llvm_profile_data *Data;

llvm/include/llvm/ProfileData/InstrProf.h

 
 /// Return the name profile runtime entry point to do value profiling
 /// for a given site.
 inline StringRef getInstrProfValueProfFuncName() {
   return INSTR_PROF_VALUE_PROF_FUNC_STR;
 }
 
 /// 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_"; }
 
 /// Return the name prefix of variables containing per-function control data.
 inline StringRef getInstrProfDataVarPrefix() { return "__profd_"; }
 
 /// Return the name prefix of profile counter variables.
 inline StringRef getInstrProfCountersVarPrefix() { return "__profc_"; }

llvm/include/llvm/ProfileData/InstrProfData.inc

 #define INSTR_PROF_COMMA ,
 #endif
 VALUE_PROF_FUNC_PARAM(uint64_t, TargetValue, Type::getInt64Ty(Ctx)) \
                       INSTR_PROF_COMMA
 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)) \
                       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))
 #endif /*VALUE_RANGE_PROF */
#define INSTR_PROF_SECT_START(Sect) \
         INSTR_PROF_CONCAT(__start_,Sect)
 #define INSTR_PROF_SECT_STOP(Sect) \
         INSTR_PROF_CONCAT(__stop_,Sect)
 
 /* Value Profiling API linkage name.  */
 #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
 
 /* The data structure that represents a tracked value by the
  * value profiler.
  */
 typedef struct InstrProfValueData {
   /* Profiled value. */
   uint64_t Value;
   /* Number of times the value appears in the training run. */
   uint64_t Count;
 } InstrProfValueData;
 
 #endif /* INSTR_PROF_DATA_INC */
 
 #ifndef INSTR_ORDER_FILE_INC
 /* The maximal # of functions: 128*1024 (the buffer size will be 128*4 KB). */
 #define INSTR_ORDER_FILE_BUFFER_SIZE 131072
 #define INSTR_ORDER_FILE_BUFFER_BITS 17
 #define INSTR_ORDER_FILE_BUFFER_MASK 0x1ffff
 #endif /* INSTR_ORDER_FILE_INC */
 #else
 #undef INSTR_PROF_DATA_DEFINED
 #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].
+ */
+
+/* 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 (__builtin_popcountll(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 - __builtin_clzll(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 (__builtin_popcountll(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 */

llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h

private:
   size_t NamesSize;
 
   // Is this lowering for the context-sensitive instrumentation.
   bool IsCS;
 
   // 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.
   int64_t MemOPSizeRangeLast;
 
   int64_t TotalCountersPromoted = 0;
 
   /// Lower instrumentation intrinsics in the function. Returns true if there

llvm/lib/ProfileData/InstrProf.cpp

bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken) {
   // For AvailableExternallyLinkage functions.
   if (!F.hasComdat()) {
     assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage);
     return true;
   }
   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) {
   static const int64_t DefaultMemOPSizeRangeStart = 0;
   static const int64_t DefaultMemOPSizeRangeLast = 8;
   RangeStart = DefaultMemOPSizeRangeStart;
   RangeLast = DefaultMemOPSizeRangeLast;
 

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

 #include <cstddef>
 #include <cstdint>
 #include <string>
 
 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(
     "memop-size-range",
     cl::desc("Set the range of size in memory intrinsic calls to be profiled "
              "precisely, in a format of <start_val>:<end_val>"),
     cl::init(""));
 
 // The value that considered to be large value in  memory intrinsic.
 cl::opt<unsigned> MemOPSizeLarge(
     "memop-size-large",
     cl::desc("Set large value thresthold in memory intrinsic size profiling. "
              "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(
     "hash-based-counter-split",
     cl::desc("Rename counter variable of a comdat function based on cfg hash"),
     cl::init(true));
 
 cl::opt<bool> RuntimeCounterRelocation(
private:
   DenseMap<Loop *, SmallVector<LoadStorePair, 8>> &LoopToCandidates;
   SmallVector<BasicBlock *, 8> ExitBlocks;
   SmallVector<Instruction *, 8> InsertPts;
   Loop &L;
   LoopInfo &LI;
   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) {
   FunctionAnalysisManager &FAM =
       AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
   auto GetTLI = [&FAM](Function &F) -> TargetLibraryInfo & {
     return FAM.getResult<TargetLibraryAnalysis>(F);
   };
bool InstrProfiling::run(
   emitVNodes();
   emitNameData();
   emitRegistration();
   emitUses();
   emitInitialization();
   return true;
 }
 
-static FunctionCallee
-getOrInsertValueProfilingCall(Module &M, const TargetLibraryInfo &TLI,
-                              bool IsRange = false) {
+static FunctionCallee getOrInsertValueProfilingCall(
+    Module &M, const TargetLibraryInfo &TLI,
+    ValueProfilingCallType CallType = ValueProfilingCallType::Default) {
   LLVMContext &Ctx = M.getContext();
   auto *ReturnTy = Type::getVoidTy(M.getContext());
 
   AttributeList AL;
   if (auto AK = TLI.getExtAttrForI32Param(false))
     AL = AL.addParamAttribute(M.getContext(), 2, AK);
 
-  if (!IsRange) {
+  if (CallType == ValueProfilingCallType::Default ||
+      CallType == ValueProfilingCallType::MemOp) {
     Type *ParamTypes[] = {
 #define VALUE_PROF_FUNC_PARAM(ParamType, ParamName, ParamLLVMType) ParamLLVMType
 #include "llvm/ProfileData/InstrProfData.inc"
     };
     auto *ValueProfilingCallTy =
         FunctionType::get(ReturnTy, makeArrayRef(ParamTypes), false);
-    return M.getOrInsertFunction(getInstrProfValueProfFuncName(),
-                                 ValueProfilingCallTy, AL);
+    StringRef FuncName = CallType == ValueProfilingCallType::Default
+                             ? getInstrProfValueProfFuncName()
+                             : getInstrProfValueProfMemOpFuncName();
+    return M.getOrInsertFunction(FuncName, 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
 #include "llvm/ProfileData/InstrProfData.inc"
 #undef VALUE_RANGE_PROF
     };
     auto *ValueRangeProfilingCallTy =
         FunctionType::get(ReturnTy, makeArrayRef(RangeParamTypes), false);
void InstrProfiling::lowerValueProfileInst(InstrProfValueProfileInst *Ind) {
 
   GlobalVariable *DataVar = It->second.DataVar;
   uint64_t ValueKind = Ind->getValueKind()->getZExtValue();
   uint64_t Index = Ind->getIndex()->getZExtValue();
   for (uint32_t Kind = IPVK_First; Kind < ValueKind; ++Kind)
     Index += It->second.NumValueSites[Kind];
 
   IRBuilder<> Builder(Ind);
-  bool IsRange = (Ind->getValueKind()->getZExtValue() ==
+  bool IsMemOpSize = (Ind->getValueKind()->getZExtValue() ==
                       llvm::InstrProfValueKind::IPVK_MemOPSize);
   CallInst *Call = nullptr;
   auto *TLI = &GetTLI(*Ind->getFunction());
 
   // To support value profiling calls within Windows exception handlers, funclet
   // information contained within operand bundles needs to be copied over to
   // the library call. This is required for the IR to be processed by the
   // WinEHPrepare pass.
   SmallVector<OperandBundleDef, 1> OpBundles;
   Ind->getOperandBundlesAsDefs(OpBundles);
-  if (!IsRange) {
+  if (!IsMemOpSize) {
     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(),
         Builder.CreateBitCast(DataVar, Builder.getInt8PtrTy()),
         Builder.getInt32(Index),
         Builder.getInt64(MemOPSizeRangeStart),
         Builder.getInt64(MemOPSizeRangeLast),
         Builder.getInt64(MemOPSizeLarge == 0 ? INT64_MIN : MemOPSizeLarge)};
-    Call = Builder.CreateCall(getOrInsertValueProfilingCall(*M, *TLI, true),
+    Call = Builder.CreateCall(getOrInsertValueProfilingCall(
+                                  *M, *TLI, ValueProfilingCallType::OldMemOp),
                               Args, OpBundles);
   }
   if (auto AK = TLI->getExtAttrForI32Param(false))
     Call->addParamAttr(2, AK);
   Ind->replaceAllUsesWith(Call);
   Ind->eraseFromParent();
 }
 

llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp

 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/Type.h"
 #include "llvm/InitializePasses.h"
 #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"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
MemOPMaxVersion("pgo-memop-max-version", cl::init(3), cl::Hidden,
                              " intrinsic calls"));
 
 // Scale the counts from the annotation using the BB count value.
 static cl::opt<bool>
     MemOPScaleCount("pgo-memop-scale-count", cl::init(true), cl::Hidden,
                     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:
   static char ID;
 
   PGOMemOPSizeOptLegacyPass() : FunctionPass(ID) {
     initializePGOMemOPSizeOptLegacyPassPass(*PassRegistry::getPassRegistry());
   }

 private:
   Function &Func;
   BlockFrequencyInfo &BFI;
   OptimizationRemarkEmitter &ORE;
   DominatorTree *DT;
   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.
   int64_t PreciseRangeLast;
   // The space to read the profile annotation.
   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.
   enum MemOPSizeKind { PreciseValue, NonLargeGroup, LargeGroup };
 
   MemOPSizeKind getMemOPSizeKind(int64_t Value) const {
     if (Value == MemOPSizeLarge && MemOPSizeLarge != 0)
       return LargeGroup;
bool MemOPSizeOpt::perform(MemOp MO) {
   // Default case is in the front -- save the slot here.
   CaseCounts.push_back(0);
   for (auto &VD : VDs) {
     int64_t V = VD.Value;
     uint64_t C = VD.Count;
     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)
+      continue;
 
     // ValueCounts are sorted on the count. Break at the first un-profitable
     // value.
     if (!isProfitable(C, RemainCount))
       break;
 
     SizeIds.push_back(V);
     CaseCounts.push_back(C);

llvm/test/Transforms/PGOProfile/memcpy.ll

-; RUN: opt < %s -pgo-instr-gen -instrprof -S | FileCheck %s
-; RUN: opt <%s -passes=pgo-instr-gen,instrprof -S | FileCheck %s
+; 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
 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"
 
 define void @foo(i8* %dst, i8* %src, i32* %a, i32 %n) {
 entry:
   br label %for.cond
 
 for.cond:
   %i.0 = phi i32 [ 0, %entry ], [ %add, %for.cond1 ]
   %cmp = icmp slt i32 %i.0, %n
   br i1 %cmp, label %for.cond1, label %for.end6
 
 for.cond1:
   %j.0 = phi i32 [ %inc, %for.body3 ], [ 0, %for.cond ]
   %idx.ext = sext i32 %i.0 to i64
   %add.ptr = getelementptr inbounds i32, i32* %a, i64 %idx.ext
   %0 = load i32, i32* %add.ptr, align 4
   %cmp2 = icmp slt i32 %j.0, %0
   %add = add nsw i32 %i.0, 1
   br i1 %cmp2, label %for.body3, label %for.cond
 
 for.body3:
   %conv = sext i32 %add to i64
-; 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)
+; 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)
   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
 
 for.end6:
   ret void
 }
 
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)

llvm/test/Transforms/PGOProfile/memop_profile_funclet.ll

 ; RUN: opt < %s -pgo-instr-gen -S | FileCheck %s --check-prefix=GEN
-; RUN: opt < %s -pgo-instr-gen -instrprof -S | FileCheck %s --check-prefix=LOWER
+; 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 -passes=pgo-instr-gen -S | FileCheck %s --check-prefix=GEN
-; RUN: opt < %s -passes=pgo-instr-gen,instrprof -S | FileCheck %s --check-prefix=LOWER
+; 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
 
 ; This test is to verify that PGO runtime library calls get created with the
 ; appropriate operand bundle funclet information when a memory intrinsic
 ; being value profiled is called within an exception handler.
 
 ; Test case based on this source:
 ;  #include <memory.h>
 ;
try.cont:                                         ; preds = %entry
   ret void
 }
 
 ; GEN: catch:
 ; GEN: call void @llvm.instrprof.value.profile(
 ; GEN-SAME: [ "funclet"(token %tmp1) ]
 
 ; LOWER: catch:
-; LOWER: call void @__llvm_profile_instrument_range(
+; LOWEROLDMEMOPVP: call void @__llvm_profile_instrument_range(
+; LOWERNEWMEMOPVP: call void @__llvm_profile_instrument_memop(
 ; LOWER-SAME: [ "funclet"(token %tmp1) ]
 
 declare dso_local void @"?may_throw@@YAXH@Z"(i32)
 declare dso_local i32 @__CxxFrameHandler3(...)
 
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8*, i64, i1)
 declare dso_local void @_CxxThrowException(i8*, %eh.ThrowInfo*)

llvm/unittests/ProfileData/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Core
   Coverage
   ProfileData
   Support
   )
 
 add_llvm_unittest(ProfileDataTests
   CoverageMappingTest.cpp
+  InstrProfDataTest.cpp
   InstrProfTest.cpp
   SampleProfTest.cpp
   )
 
 target_link_libraries(ProfileDataTests PRIVATE LLVMTestingSupport)

llvm/unittests/ProfileData/InstrProfDataTest.cpp

+//===- 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