Skip to content

Commit 3c4e85b

Browse files
wlei-llvmwlei-llvm
committed
addressing comments
1 parent b1f0105 commit 3c4e85b

File tree

6 files changed

+40
-50
lines changed

6 files changed

+40
-50
lines changed

‎clang/include/clang/Driver/Options.td‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1786,10 +1786,10 @@ defm debug_info_for_profiling : BoolFOption<"debug-info-for-profiling",
17861786
NegFlag<SetFalse>>;
17871787
def fprofile_generate_cold_function_coverage : Flag<["-"], "fprofile-generate-cold-function-coverage">,
17881788
Group<f_Group>, Visibility<[ClangOption, CLOption]>,
1789-
HelpText<"Generate instrumented code to cold functions into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;
1789+
HelpText<"Generate instrumented code to collect coverage info for cold functions into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;
17901790
def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], "fprofile-generate-cold-function-coverage=">,
17911791
Group<f_Group>, Visibility<[ClangOption, CLOption]>, MetaVarName<"<directory>">,
1792-
HelpText<"Generate instrumented code to cold functions into <directory>/default.profraw (overridden by LLVM_PROFILE_FILE env var)">;
1792+
HelpText<"Generate instrumented code to collect coverage info for cold functions into <directory>/default.profraw (overridden by LLVM_PROFILE_FILE env var)">;
17931793
def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
17941794
Group<f_Group>, Visibility<[ClangOption, CLOption]>,
17951795
HelpText<"Generate instrumented code to collect execution counts into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;

‎clang/lib/Driver/ToolChains/Clang.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -658,11 +658,13 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
658658
? ColdFuncCoverageArg->getValue()
659659
: "");
660660
llvm::sys::path::append(Path, "default_%m.profraw");
661+
// FIXME: Idealy the file path should be passed through
662+
// `-fprofile-instrument-path=`(InstrProfileOutput), however, this field is
663+
// shared with other profile use path(see PGOOptions), we need to refactor
664+
// PGOOptions to make it work.
661665
CmdArgs.push_back("-mllvm");
662666
CmdArgs.push_back(Args.MakeArgString(
663-
Twine("--instrument-cold-function-coverage-path=") + Path));
664-
CmdArgs.push_back("-mllvm");
665-
CmdArgs.push_back("--instrument-cold-function-coverage");
667+
Twine("--instrument-cold-function-only-path=") + Path));
666668
CmdArgs.push_back("-mllvm");
667669
CmdArgs.push_back("--pgo-function-entry-coverage");
668670
}

‎clang/test/Driver/fprofile-generate-cold-function-coverage.c‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
// RUN: %clang -### -c -fprofile-generate-cold-function-coverage %s 2>&1 | FileCheck %s
2-
// CHECK: "--instrument-cold-function-coverage-path=default_%m.profraw"
3-
// CHECK: "--instrument-cold-function-coverage"
2+
// CHECK: "--instrument-cold-function-only-path=default_%m.profraw"
43
// CHECK: "--pgo-function-entry-coverage"
54
// CHECK-NOT: "-fprofile-instrument"
65
// CHECK-NOT: "-fprofile-instrument-path=
76

87
// RUN: %clang -### -c -fprofile-generate-cold-function-coverage=dir %s 2>&1 | FileCheck %s --check-prefix=CHECK-EQ
9-
// CHECK-EQ: "--instrument-cold-function-coverage-path=dir{{/|\\\\}}default_%m.profraw"
8+
// CHECK-EQ: "--instrument-cold-function-only-path=dir{{/|\\\\}}default_%m.profraw"

‎llvm/lib/Passes/PassBuilderPipelines.cpp‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -296,13 +296,12 @@ static cl::opt<bool> UseLoopVersioningLICM(
296296
"enable-loop-versioning-licm", cl::init(false), cl::Hidden,
297297
cl::desc("Enable the experimental Loop Versioning LICM pass"));
298298

299-
static cl::opt<std::string> InstrumentColdFuncCoveragePath(
300-
"instrument-cold-function-coverage-path", cl::init(""),
301-
cl::desc("File path for cold function coverage instrumentation"),
302-
cl::Hidden);
299+
static cl::opt<std::string> InstrumentColdFuncOnlyPath(
300+
"instrument-cold-function-only-path", cl::init(""),
301+
cl::desc("File path for cold function only instrumentation"), cl::Hidden);
303302

304303
extern cl::opt<std::string> UseCtxProfile;
305-
extern cl::opt<bool> InstrumentColdFunctionCoverage;
304+
extern cl::opt<bool> PGOInstrumentColdFunctionOnly;
306305

307306
namespace llvm {
308307
extern cl::opt<bool> EnableMemProfContextDisambiguation;
@@ -1191,12 +1190,12 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
11911190
!UseCtxProfile.empty() && Phase == ThinOrFullLTOPhase::ThinLTOPreLink;
11921191

11931192
// Enable cold function coverage instrumentation if
1194-
// InstrumentColdFuncCoveragePath is provided.
1195-
const bool IsColdFuncCoverageGen = InstrumentColdFunctionCoverage =
1196-
IsPGOPreLink && !InstrumentColdFuncCoveragePath.empty();
1193+
// InstrumentColdFuncOnlyPath is provided.
1194+
const bool IsColdFuncOnlyInstrGen = PGOInstrumentColdFunctionOnly =
1195+
IsPGOPreLink && !InstrumentColdFuncOnlyPath.empty();
11971196

11981197
if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen ||
1199-
IsCtxProfUse || IsColdFuncCoverageGen)
1198+
IsCtxProfUse || IsColdFuncOnlyInstrGen)
12001199
addPreInlinerPasses(MPM, Level, Phase);
12011200

12021201
// Add all the requested passes for instrumentation PGO, if requested.
@@ -1218,10 +1217,10 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
12181217
return MPM;
12191218
addPostPGOLoopRotation(MPM, Level);
12201219
MPM.addPass(PGOCtxProfLoweringPass());
1221-
} else if (IsColdFuncCoverageGen) {
1220+
} else if (IsColdFuncOnlyInstrGen) {
12221221
addPGOInstrPasses(
12231222
MPM, Level, /* RunProfileGen */ true, /* IsCS */ false,
1224-
/* AtomicCounterUpdate */ false, InstrumentColdFuncCoveragePath,
1223+
/* AtomicCounterUpdate */ false, InstrumentColdFuncOnlyPath,
12251224
/* ProfileRemappingFile */ "", IntrusiveRefCntPtr<vfs::FileSystem>());
12261225
}
12271226

‎llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp‎

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -319,28 +319,19 @@ static cl::opt<unsigned> PGOFunctionCriticalEdgeThreshold(
319319
cl::desc("Do not instrument functions with the number of critical edges "
320320
" greater than this threshold."));
321321

322-
static cl::opt<uint64_t> ColdFuncCoverageMaxEntryCount(
323-
"cold-function-coverage-max-entry-count", cl::init(0), cl::Hidden,
324-
cl::desc("When enabling cold function coverage instrumentation, skip "
325-
"instrumenting the function whose entry count is above the given "
326-
"value"));
327-
328-
static cl::opt<InstrColdFuncCovMode> InstrumentColdFunctionCoverageMode(
329-
"instrument-cold-function-coverage-mode",
330-
cl::init(InstrColdFuncCovMode::Conservative), cl::Hidden,
331-
cl::desc("Control whether to instrument unprofiled functions for cold "
332-
"function coverage."),
333-
cl::values(
334-
clEnumValN(InstrColdFuncCovMode::Conservative, "conservative",
335-
"Assume unprofiled functions are not cold, skip "
336-
"instrumenting them."),
337-
clEnumValN(InstrColdFuncCovMode::Optimistic, "optimistic",
338-
"Treat unprofiled functions as cold and instrument them.")));
339-
340-
cl::opt<bool> InstrumentColdFunctionCoverage(
341-
"instrument-cold-function-coverage", cl::init(false), cl::Hidden,
342-
cl::desc("Enable cold function coverage instrumentation (currently only "
343-
"used under sampling PGO pipeline)"));
322+
static cl::opt<uint64_t> PGOColdInstrumentEntryThreshold(
323+
"pgo-cold-instrument-entry-threshold", cl::init(0), cl::Hidden,
324+
cl::desc("For cold function instrumentation, skip instrumenting functions "
325+
"whose entry count is above the given value."));
326+
327+
static cl::opt<bool> PGOTreatUnknownAsCold(
328+
"pgo-treat-unknown-as-cold", cl::init(false), cl::Hidden,
329+
cl::desc("For cold function instrumentation, treat count unknown(e.g. "
330+
"unprofiled) functions as cold."));
331+
332+
cl::opt<bool> PGOInstrumentColdFunctionOnly(
333+
"pgo-instrument-cold-function-only", cl::init(false), cl::Hidden,
334+
cl::desc("Enable cold function only instrumentation."));
344335

345336
extern cl::opt<unsigned> MaxNumVTableAnnotations;
346337

@@ -1914,11 +1905,10 @@ static bool skipPGOGen(const Function &F) {
19141905
return true;
19151906
if (F.getInstructionCount() < PGOFunctionSizeThreshold)
19161907
return true;
1917-
if (InstrumentColdFunctionCoverage) {
1908+
if (PGOInstrumentColdFunctionOnly) {
19181909
if (auto EntryCount = F.getEntryCount())
1919-
return EntryCount->getCount() > ColdFuncCoverageMaxEntryCount;
1920-
return InstrumentColdFunctionCoverageMode ==
1921-
InstrColdFuncCovMode::Conservative;
1910+
return EntryCount->getCount() > PGOColdInstrumentEntryThreshold;
1911+
return !PGOTreatUnknownAsCold;
19221912
}
19231913
return false;
19241914
}

‎llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -pgo-function-entry-coverage -S | FileCheck --check-prefixes=COLD %s
2-
; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -pgo-function-entry-coverage -cold-function-coverage-max-entry-count=1 -S | FileCheck --check-prefixes=ENTRY-COUNT %s
3-
; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -pgo-function-entry-coverage -instrument-cold-function-coverage-mode=optimistic -S | FileCheck --check-prefixes=UNPROFILED-FUNC %s
1+
; RUN: opt < %s --passes=pgo-instr-gen -pgo-instrument-cold-function-only -pgo-function-entry-coverage -S | FileCheck --check-prefixes=COLD %s
2+
; RUN: opt < %s --passes=pgo-instr-gen -pgo-instrument-cold-function-only -pgo-function-entry-coverage -pgo-cold-instrument-entry-threshold=1 -S | FileCheck --check-prefixes=ENTRY-COUNT %s
3+
; RUN: opt < %s --passes=pgo-instr-gen -pgo-instrument-cold-function-only -pgo-function-entry-coverage -pgo-treat-unknown-as-cold -S | FileCheck --check-prefixes=UNKNOWN-FUNC %s
44

55
; COLD: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0)
66
; COLD-NOT: __profn_main
@@ -9,8 +9,8 @@
99
; ENTRY-COUNT: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0)
1010
; ENTRY-COUNT: call void @llvm.instrprof.cover(ptr @__profn_main, i64 [[#]], i32 1, i32 0)
1111

12-
; UNPROFILED-FUNC: call void @llvm.instrprof.cover(ptr @__profn_bar, i64 [[#]], i32 1, i32 0)
13-
; UNPROFILED-FUNC: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0)
12+
; UNKNOWN-FUNC: call void @llvm.instrprof.cover(ptr @__profn_bar, i64 [[#]], i32 1, i32 0)
13+
; UNKNOWN-FUNC: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0)
1414

1515

1616
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"

0 commit comments

Comments
 (0)