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

Hiroshi Yamauchi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 22 11:49:39 PDT 2020


yamauchi added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:620
   } else {
+    // FIXME: This code is to be removed after switching to the new memop value
+    // profiling.
----------------
davidxl wrote:
> this branch can be merged with the above -- though it is to be deprecated.
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.)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81682





More information about the llvm-commits mailing list