https://github.com/wlei-llvm updated https://github.com/llvm/llvm-project/pull/109837
>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Sun, 22 Sep 2024 20:23:20 -0700 Subject: [PATCH 1/6] [InstrPGO] Instrument sampling profile based cold function --- clang/include/clang/Driver/Options.td | 6 +++++ clang/lib/Driver/ToolChain.cpp | 4 +++- clang/lib/Driver/ToolChains/Clang.cpp | 18 ++++++++++++++ clang/test/CodeGen/Inputs/pgo-cold-func.prof | 2 ++ .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++++++++++ llvm/lib/Passes/PassBuilderPipelines.cpp | 18 ++++++++++++++ .../Instrumentation/PGOInstrumentation.cpp | 16 +++++++++++++ .../PGOProfile/instr-gen-cold-function.ll | 24 +++++++++++++++++++ 8 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 002f60350543d9..6bb92427f2d53f 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : BoolFOption<"debug-info-for-profiling", PosFlag<SetTrue, [], [ClangOption, CC1Option], "Emit extra debug info to make sample profile more accurate">, NegFlag<SetFalse>>; +def fprofile_generate_cold_function_coverage : Flag<["-"], "fprofile-generate-cold-function-coverage">, + Group<f_Group>, Visibility<[ClangOption, CLOption]>, + HelpText<"Generate instrumented code to cold functions into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">; +def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], "fprofile-generate-cold-function-coverage=">, + Group<f_Group>, Visibility<[ClangOption, CLOption]>, MetaVarName<"<directory>">, + HelpText<"Generate instrumented code to cold functions into <directory>/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">, Group<f_Group>, Visibility<[ClangOption, CLOption]>, HelpText<"Generate instrumented code to collect execution counts into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">; diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp index 16f9b629fc538c..e56db150c6b47e 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) { Args.hasArg(options::OPT_fprofile_instr_generate) || Args.hasArg(options::OPT_fprofile_instr_generate_EQ) || Args.hasArg(options::OPT_fcreate_profile) || - Args.hasArg(options::OPT_forder_file_instrumentation); + Args.hasArg(options::OPT_forder_file_instrumentation) || + Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) || + Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ); } bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) { diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 0bab48caf1a5e2..00602a08232ba2 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -649,6 +649,24 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C, } } + if (auto *ColdFuncCoverageArg = Args.getLastArg( + options::OPT_fprofile_generate_cold_function_coverage, + options::OPT_fprofile_generate_cold_function_coverage_EQ)) { + SmallString<128> Path( + ColdFuncCoverageArg->getOption().matches( + options::OPT_fprofile_generate_cold_function_coverage_EQ) + ? ColdFuncCoverageArg->getValue() + : ""); + llvm::sys::path::append(Path, "default_%m.profraw"); + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back(Args.MakeArgString( + Twine("--instrument-sample-cold-function-path=") + Path)); + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back("--instrument-cold-function-coverage"); + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back("--pgo-function-entry-coverage"); + } + Arg *PGOGenArg = nullptr; if (PGOGenerateArg) { assert(!CSPGOGenerateArg); diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof b/clang/test/CodeGen/Inputs/pgo-cold-func.prof new file mode 100644 index 00000000000000..e50be02e0a8545 --- /dev/null +++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof @@ -0,0 +1,2 @@ +foo:1:1 + 1: 1 diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c b/clang/test/CodeGen/pgo-cold-function-coverage.c new file mode 100644 index 00000000000000..0d2767c022b9f1 --- /dev/null +++ b/clang/test/CodeGen/pgo-cold-function-coverage.c @@ -0,0 +1,12 @@ +// Test -fprofile-generate-cold-function-coverage +// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx/yyy/ -fprofile-sample-accurate -fprofile-sample-use=%S/Inputs/pgo-cold-func.prof -S -emit-llvm -o - %s | FileCheck %s + +// CHECK: @__llvm_profile_filename = {{.*}} c"/xxx/yyy/default_%m.profraw\00" + +// CHECK: @__profc_bar +// CHECK-NOT: @__profc_foo + +int bar(int x) { return x;} +int foo(int x) { + return x; +} diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index 8f151a99b11709..f75abe0c7c7649 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -296,7 +296,13 @@ static cl::opt<bool> UseLoopVersioningLICM( "enable-loop-versioning-licm", cl::init(false), cl::Hidden, cl::desc("Enable the experimental Loop Versioning LICM pass")); +static cl::opt<std::string> InstrumentSampleColdFuncPath( + "instrument-sample-cold-function-path", cl::init(""), + cl::desc("File path for instrumenting sampling PGO guided cold functions"), + cl::Hidden); + extern cl::opt<std::string> UseCtxProfile; +extern cl::opt<bool> InstrumentColdFunctionCoverage; namespace llvm { extern cl::opt<bool> EnableMemProfContextDisambiguation; @@ -1119,6 +1125,18 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, // removed. MPM.addPass( PGOIndirectCallPromotion(true /* IsInLTO */, true /* SamplePGO */)); + + if (InstrumentSampleColdFuncPath.getNumOccurrences() && + Phase != ThinOrFullLTOPhase::ThinLTOPostLink) { + assert(!InstrumentSampleColdFuncPath.empty() && + "File path is requeired for instrumentation generation"); + InstrumentColdFunctionCoverage = true; + addPreInlinerPasses(MPM, Level, Phase); + addPGOInstrPasses(MPM, Level, /* RunProfileGen */ true, + /* IsCS */ false, /* AtomicCounterUpdate */ false, + InstrumentSampleColdFuncPath, "", + IntrusiveRefCntPtr<vfs::FileSystem>()); + } } // Try to perform OpenMP specific optimizations on the module. This is a diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index 10442fa0bb9003..890ad853d86461 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -319,6 +319,18 @@ static cl::opt<unsigned> PGOFunctionCriticalEdgeThreshold( cl::desc("Do not instrument functions with the number of critical edges " " greater than this threshold.")); +cl::opt<bool> InstrumentColdFunctionCoverage( + "instrument-cold-function-coverage", cl::init(false), cl::Hidden, + cl::desc("Enable cold function coverage instrumentation (currently only " + "used under sampling " + " PGO pipeline))")); + +static cl::opt<uint64_t> ColdFuncCoverageMaxEntryCount( + "cold-function-coverage-max-entry-count", cl::init(0), cl::Hidden, + cl::desc("When enabling cold function coverage instrumentation, skip " + "instrumenting the " + "function whose entry count is above the given value")); + extern cl::opt<unsigned> MaxNumVTableAnnotations; namespace llvm { @@ -1891,6 +1903,10 @@ static bool skipPGOGen(const Function &F) { return true; if (F.getInstructionCount() < PGOFunctionSizeThreshold) return true; + if (InstrumentColdFunctionCoverage && + (!F.getEntryCount() || + F.getEntryCount()->getCount() > ColdFuncCoverageMaxEntryCount)) + return true; return false; } diff --git a/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll b/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll new file mode 100644 index 00000000000000..ba1a11f7184356 --- /dev/null +++ b/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll @@ -0,0 +1,24 @@ +; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -S | FileCheck --check-prefixes=COLD %s +; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -cold-function-coverage-max-entry-count=1 -S | FileCheck --check-prefixes=ENTRY-COUNT %s + +; COLD: call void @llvm.instrprof.increment(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) +; COLD-NOT: __profn_main + +; ENTRY-COUNT: call void @llvm.instrprof.increment(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) +; ENTRY-COUNT: call void @llvm.instrprof.increment(ptr @__profn_main, i64 [[#]], i32 1, i32 0) + +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" +target triple = "x86_64-unknown-linux-gnu" + +define void @foo() !prof !0 { +entry: + ret void +} + +define i32 @main() !prof !1 { +entry: + ret i32 0 +} + +!0 = !{!"function_entry_count", i64 0} +!1 = !{!"function_entry_count", i64 1} >From b3054f05007d60900cb02301cf59711c68e70120 Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Tue, 1 Oct 2024 15:51:37 -0700 Subject: [PATCH 2/6] fix lint in cl desc msg --- .../Instrumentation/PGOInstrumentation.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index 890ad853d86461..aba1d8cc88c224 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -319,17 +319,16 @@ static cl::opt<unsigned> PGOFunctionCriticalEdgeThreshold( cl::desc("Do not instrument functions with the number of critical edges " " greater than this threshold.")); -cl::opt<bool> InstrumentColdFunctionCoverage( - "instrument-cold-function-coverage", cl::init(false), cl::Hidden, - cl::desc("Enable cold function coverage instrumentation (currently only " - "used under sampling " - " PGO pipeline))")); - static cl::opt<uint64_t> ColdFuncCoverageMaxEntryCount( "cold-function-coverage-max-entry-count", cl::init(0), cl::Hidden, cl::desc("When enabling cold function coverage instrumentation, skip " - "instrumenting the " - "function whose entry count is above the given value")); + "instrumenting the function whose entry count is above the given " + "value")); + +cl::opt<bool> InstrumentColdFunctionCoverage( + "instrument-cold-function-coverage", cl::init(false), cl::Hidden, + cl::desc("Enable cold function coverage instrumentation (currently only " + "used under sampling PGO pipeline)")); extern cl::opt<unsigned> MaxNumVTableAnnotations; >From 5a2848cd196d167570c4067ff171cc038a70c762 Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Wed, 2 Oct 2024 10:13:52 -0700 Subject: [PATCH 3/6] address comments, ad fix test, add driver test, refactoring the addPGOInstrPasses --- clang/test/CodeGen/Inputs/pgo-cold-func.prof | 2 -- .../test/CodeGen/pgo-cold-function-coverage.c | 13 +++++++-- ...fprofile-generate-cold-function-coverage.c | 9 ++++++ llvm/lib/Passes/PassBuilderPipelines.cpp | 28 ++++++++++--------- .../PGOProfile/instr-gen-cold-function.ll | 10 +++---- 5 files changed, 39 insertions(+), 23 deletions(-) delete mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof create mode 100644 clang/test/Driver/fprofile-generate-cold-function-coverage.c diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof b/clang/test/CodeGen/Inputs/pgo-cold-func.prof deleted file mode 100644 index e50be02e0a8545..00000000000000 --- a/clang/test/CodeGen/Inputs/pgo-cold-func.prof +++ /dev/null @@ -1,2 +0,0 @@ -foo:1:1 - 1: 1 diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c b/clang/test/CodeGen/pgo-cold-function-coverage.c index 0d2767c022b9f1..43ddc125a56ee7 100644 --- a/clang/test/CodeGen/pgo-cold-function-coverage.c +++ b/clang/test/CodeGen/pgo-cold-function-coverage.c @@ -1,11 +1,18 @@ // Test -fprofile-generate-cold-function-coverage -// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx/yyy/ -fprofile-sample-accurate -fprofile-sample-use=%S/Inputs/pgo-cold-func.prof -S -emit-llvm -o - %s | FileCheck %s + +// RUN: split-file %s %t +// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx/yyy/ -fprofile-sample-accurate -fprofile-sample-use=%t/pgo-cold-func.prof -S -emit-llvm -o - %t/pgo-cold-func.c | FileCheck %s // CHECK: @__llvm_profile_filename = {{.*}} c"/xxx/yyy/default_%m.profraw\00" -// CHECK: @__profc_bar -// CHECK-NOT: @__profc_foo +// CHECK: store i8 0, ptr @__profc_bar, align 1 +// CHECK-NOT: @__profc_foo + +//--- pgo-cold-func.prof +foo:1:1 + 1: 1 +//--- pgo-cold-func.c int bar(int x) { return x;} int foo(int x) { return x; diff --git a/clang/test/Driver/fprofile-generate-cold-function-coverage.c b/clang/test/Driver/fprofile-generate-cold-function-coverage.c new file mode 100644 index 00000000000000..76b1ee4353cdbf --- /dev/null +++ b/clang/test/Driver/fprofile-generate-cold-function-coverage.c @@ -0,0 +1,9 @@ +// RUN: %clang -### -c -fprofile-generate-cold-function-coverage %s 2>&1 | FileCheck %s +// CHECK: "--instrument-sample-cold-function-path=default_%m.profraw" +// CHECK: "--instrument-cold-function-coverage" +// CHECK: "--pgo-function-entry-coverage" +// CHECK-NOT: "-fprofile-instrument" +// CHECK-NOT: "-fprofile-instrument-path= + +// RUN: %clang -### -c -fprofile-generate-cold-function-coverage=dir %s 2>&1 | FileCheck %s --check-prefix=CHECK-PATH +// CHECK-PATH: "--instrument-sample-cold-function-path=dir{{/|\\\\}}default_%m.profraw" diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index f75abe0c7c7649..be543708a8fe29 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -1125,18 +1125,6 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, // removed. MPM.addPass( PGOIndirectCallPromotion(true /* IsInLTO */, true /* SamplePGO */)); - - if (InstrumentSampleColdFuncPath.getNumOccurrences() && - Phase != ThinOrFullLTOPhase::ThinLTOPostLink) { - assert(!InstrumentSampleColdFuncPath.empty() && - "File path is requeired for instrumentation generation"); - InstrumentColdFunctionCoverage = true; - addPreInlinerPasses(MPM, Level, Phase); - addPGOInstrPasses(MPM, Level, /* RunProfileGen */ true, - /* IsCS */ false, /* AtomicCounterUpdate */ false, - InstrumentSampleColdFuncPath, "", - IntrusiveRefCntPtr<vfs::FileSystem>()); - } } // Try to perform OpenMP specific optimizations on the module. This is a @@ -1202,8 +1190,17 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, const bool IsCtxProfUse = !UseCtxProfile.empty() && Phase == ThinOrFullLTOPhase::ThinLTOPreLink; + // Enable sampling-based cold function coverage instrumentation if + // InstrumentSampleColdFuncPath is provided. + const bool IsSampleColdFuncCovGen = InstrumentColdFunctionCoverage = + IsPGOPreLink && LoadSampleProfile && + !InstrumentSampleColdFuncPath.empty(); + assert((InstrumentSampleColdFuncPath.empty() || LoadSampleProfile) && + "Sampling-based cold functon coverage is not supported without " + "providing sampling PGO profile"); + if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen || - IsCtxProfUse) + IsCtxProfUse || IsSampleColdFuncCovGen) addPreInlinerPasses(MPM, Level, Phase); // Add all the requested passes for instrumentation PGO, if requested. @@ -1225,6 +1222,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, return MPM; addPostPGOLoopRotation(MPM, Level); MPM.addPass(PGOCtxProfLoweringPass()); + } else if (IsSampleColdFuncCovGen) { + addPGOInstrPasses( + MPM, Level, /* RunProfileGen */ true, /* IsCS */ false, + /* AtomicCounterUpdate */ false, InstrumentSampleColdFuncPath, + /* ProfileRemappingFile */ "", IntrusiveRefCntPtr<vfs::FileSystem>()); } if (IsPGOInstrGen || IsPGOInstrUse || IsCtxProfGen) diff --git a/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll b/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll index ba1a11f7184356..336fceef8d3a6b 100644 --- a/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll +++ b/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll @@ -1,11 +1,11 @@ -; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -S | FileCheck --check-prefixes=COLD %s -; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -cold-function-coverage-max-entry-count=1 -S | FileCheck --check-prefixes=ENTRY-COUNT %s +; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -pgo-function-entry-coverage -S | FileCheck --check-prefixes=COLD %s +; 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 -; COLD: call void @llvm.instrprof.increment(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) +; COLD: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) ; COLD-NOT: __profn_main -; ENTRY-COUNT: call void @llvm.instrprof.increment(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) -; ENTRY-COUNT: call void @llvm.instrprof.increment(ptr @__profn_main, i64 [[#]], i32 1, i32 0) +; ENTRY-COUNT: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) +; ENTRY-COUNT: call void @llvm.instrprof.cover(ptr @__profn_main, i64 [[#]], i32 1, i32 0) 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" target triple = "x86_64-unknown-linux-gnu" >From f07457f0dd959503a6f16ab301e909e79eaffd7f Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Tue, 8 Oct 2024 10:04:51 -0700 Subject: [PATCH 4/6] make it general, remove sample from vaiable name --- llvm/lib/Passes/PassBuilderPipelines.cpp | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index be543708a8fe29..21594a8000bcfa 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -296,9 +296,9 @@ static cl::opt<bool> UseLoopVersioningLICM( "enable-loop-versioning-licm", cl::init(false), cl::Hidden, cl::desc("Enable the experimental Loop Versioning LICM pass")); -static cl::opt<std::string> InstrumentSampleColdFuncPath( - "instrument-sample-cold-function-path", cl::init(""), - cl::desc("File path for instrumenting sampling PGO guided cold functions"), +static cl::opt<std::string> InstrumentColdFuncCoveragePath( + "instrument-cold-function-coverage-path", cl::init(""), + cl::desc("File path for cold function coverage instrumentation"));, cl::Hidden); extern cl::opt<std::string> UseCtxProfile; @@ -1190,17 +1190,13 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, const bool IsCtxProfUse = !UseCtxProfile.empty() && Phase == ThinOrFullLTOPhase::ThinLTOPreLink; - // Enable sampling-based cold function coverage instrumentation if - // InstrumentSampleColdFuncPath is provided. - const bool IsSampleColdFuncCovGen = InstrumentColdFunctionCoverage = - IsPGOPreLink && LoadSampleProfile && - !InstrumentSampleColdFuncPath.empty(); - assert((InstrumentSampleColdFuncPath.empty() || LoadSampleProfile) && - "Sampling-based cold functon coverage is not supported without " - "providing sampling PGO profile"); + // Enable cold function coverage instrumentation if + // InstrumentColdFuncCoveragePath is provided. + const bool IsColdFuncCoverageGen = InstrumentColdFunctionCoverage = + IsPGOPreLink && !InstrumentColdFuncCoveragePath.empty(); if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen || - IsCtxProfUse || IsSampleColdFuncCovGen) + IsCtxProfUse || IsColdFuncCoverageGen) addPreInlinerPasses(MPM, Level, Phase); // Add all the requested passes for instrumentation PGO, if requested. @@ -1222,10 +1218,10 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, return MPM; addPostPGOLoopRotation(MPM, Level); MPM.addPass(PGOCtxProfLoweringPass()); - } else if (IsSampleColdFuncCovGen) { + } else if (IsColdFuncCoverageGen) { addPGOInstrPasses( MPM, Level, /* RunProfileGen */ true, /* IsCS */ false, - /* AtomicCounterUpdate */ false, InstrumentSampleColdFuncPath, + /* AtomicCounterUpdate */ false, InstrumentColdFuncCoveragePath, /* ProfileRemappingFile */ "", IntrusiveRefCntPtr<vfs::FileSystem>()); } >From 66f837a602c62d98aacff82422daa5182ac6d13e Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Tue, 8 Oct 2024 14:59:16 -0700 Subject: [PATCH 5/6] add a flag to control unprofiled function --- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- ...fprofile-generate-cold-function-coverage.c | 6 ++--- .../Instrumentation/PGOInstrumentation.h | 2 ++ llvm/lib/Passes/PassBuilderPipelines.cpp | 2 +- .../Instrumentation/PGOInstrumentation.cpp | 22 +++++++++++++++---- .../PGOProfile/instr-gen-cold-function.ll | 11 ++++++++++ 6 files changed, 36 insertions(+), 9 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 00602a08232ba2..306a0ecfed583a 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -660,7 +660,7 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C, llvm::sys::path::append(Path, "default_%m.profraw"); CmdArgs.push_back("-mllvm"); CmdArgs.push_back(Args.MakeArgString( - Twine("--instrument-sample-cold-function-path=") + Path)); + Twine("--instrument-cold-function-coverage-path=") + Path)); CmdArgs.push_back("-mllvm"); CmdArgs.push_back("--instrument-cold-function-coverage"); CmdArgs.push_back("-mllvm"); diff --git a/clang/test/Driver/fprofile-generate-cold-function-coverage.c b/clang/test/Driver/fprofile-generate-cold-function-coverage.c index 76b1ee4353cdbf..7b431b02583abf 100644 --- a/clang/test/Driver/fprofile-generate-cold-function-coverage.c +++ b/clang/test/Driver/fprofile-generate-cold-function-coverage.c @@ -1,9 +1,9 @@ // RUN: %clang -### -c -fprofile-generate-cold-function-coverage %s 2>&1 | FileCheck %s -// CHECK: "--instrument-sample-cold-function-path=default_%m.profraw" +// CHECK: "--instrument-cold-function-coverage-path=default_%m.profraw" // CHECK: "--instrument-cold-function-coverage" // CHECK: "--pgo-function-entry-coverage" // CHECK-NOT: "-fprofile-instrument" // CHECK-NOT: "-fprofile-instrument-path= -// RUN: %clang -### -c -fprofile-generate-cold-function-coverage=dir %s 2>&1 | FileCheck %s --check-prefix=CHECK-PATH -// CHECK-PATH: "--instrument-sample-cold-function-path=dir{{/|\\\\}}default_%m.profraw" +// RUN: %clang -### -c -fprofile-generate-cold-function-coverage=dir %s 2>&1 | FileCheck %s --check-prefix=CHECK-EQ +// CHECK-EQ: "--instrument-cold-function-coverage-path=dir{{/|\\\\}}default_%m.profraw" diff --git a/llvm/include/llvm/Transforms/Instrumentation/PGOInstrumentation.h b/llvm/include/llvm/Transforms/Instrumentation/PGOInstrumentation.h index 1d0af87e965af5..2911da3a628c00 100644 --- a/llvm/include/llvm/Transforms/Instrumentation/PGOInstrumentation.h +++ b/llvm/include/llvm/Transforms/Instrumentation/PGOInstrumentation.h @@ -53,6 +53,8 @@ class PGOInstrumentationGenCreateVar bool ProfileSampling; }; +enum class InstrColdFuncCovMode { Conservative = 0, Optimistic }; + enum class PGOInstrumentationType { Invalid = 0, FDO, CSFDO, CTXPROF }; /// The instrumentation (profile-instr-gen) pass for IR based PGO. class PGOInstrumentationGen : public PassInfoMixin<PGOInstrumentationGen> { diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index 21594a8000bcfa..b9f1a0b0d0efaf 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -298,7 +298,7 @@ static cl::opt<bool> UseLoopVersioningLICM( static cl::opt<std::string> InstrumentColdFuncCoveragePath( "instrument-cold-function-coverage-path", cl::init(""), - cl::desc("File path for cold function coverage instrumentation"));, + cl::desc("File path for cold function coverage instrumentation"), cl::Hidden); extern cl::opt<std::string> UseCtxProfile; diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index aba1d8cc88c224..f7ca6bafa82198 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -325,6 +325,18 @@ static cl::opt<uint64_t> ColdFuncCoverageMaxEntryCount( "instrumenting the function whose entry count is above the given " "value")); +static cl::opt<InstrColdFuncCovMode> InstrumentColdFunctionCoverageMode( + "instrument-cold-function-coverage-mode", + cl::init(InstrColdFuncCovMode::Conservative), cl::Hidden, + cl::desc("Control whether instrumenting unprofiled functions for cold " + "function coverage."), + cl::values( + clEnumValN(InstrColdFuncCovMode::Conservative, "conservative", + "Assume unprofiled functions are not cold, skip " + "instrumenting them."), + clEnumValN(InstrColdFuncCovMode::Optimistic, "optimistic", + "Treat unprofiled functions as cold and instrument them."))); + cl::opt<bool> InstrumentColdFunctionCoverage( "instrument-cold-function-coverage", cl::init(false), cl::Hidden, cl::desc("Enable cold function coverage instrumentation (currently only " @@ -1902,10 +1914,12 @@ static bool skipPGOGen(const Function &F) { return true; if (F.getInstructionCount() < PGOFunctionSizeThreshold) return true; - if (InstrumentColdFunctionCoverage && - (!F.getEntryCount() || - F.getEntryCount()->getCount() > ColdFuncCoverageMaxEntryCount)) - return true; + if (InstrumentColdFunctionCoverage) { + if (!F.getEntryCount()) + return InstrumentColdFunctionCoverageMode == + InstrColdFuncCovMode::Conservative; + return F.getEntryCount()->getCount() > ColdFuncCoverageMaxEntryCount; + } return false; } diff --git a/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll b/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll index 336fceef8d3a6b..a7e59bd665080b 100644 --- a/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll +++ b/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll @@ -1,15 +1,26 @@ ; RUN: opt < %s --passes=pgo-instr-gen -instrument-cold-function-coverage -pgo-function-entry-coverage -S | FileCheck --check-prefixes=COLD %s ; 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 +; 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 ; COLD: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) ; COLD-NOT: __profn_main +; COLD-NOT: __profn_bar ; ENTRY-COUNT: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) ; ENTRY-COUNT: call void @llvm.instrprof.cover(ptr @__profn_main, i64 [[#]], i32 1, i32 0) +; UNPROFILED-FUNC: call void @llvm.instrprof.cover(ptr @__profn_bar, i64 [[#]], i32 1, i32 0) +; UNPROFILED-FUNC: call void @llvm.instrprof.cover(ptr @__profn_foo, i64 [[#]], i32 1, i32 0) + + 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" target triple = "x86_64-unknown-linux-gnu" +define void @bar() { +entry: + ret void +} + define void @foo() !prof !0 { entry: ret void >From b1f0105be99278f21efee94f8486d2a082385133 Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Wed, 9 Oct 2024 12:00:24 -0700 Subject: [PATCH 6/6] address comments --- .../Transforms/Instrumentation/PGOInstrumentation.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index f7ca6bafa82198..13986031c06f3e 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -328,7 +328,7 @@ static cl::opt<uint64_t> ColdFuncCoverageMaxEntryCount( static cl::opt<InstrColdFuncCovMode> InstrumentColdFunctionCoverageMode( "instrument-cold-function-coverage-mode", cl::init(InstrColdFuncCovMode::Conservative), cl::Hidden, - cl::desc("Control whether instrumenting unprofiled functions for cold " + cl::desc("Control whether to instrument unprofiled functions for cold " "function coverage."), cl::values( clEnumValN(InstrColdFuncCovMode::Conservative, "conservative", @@ -1915,10 +1915,10 @@ static bool skipPGOGen(const Function &F) { if (F.getInstructionCount() < PGOFunctionSizeThreshold) return true; if (InstrumentColdFunctionCoverage) { - if (!F.getEntryCount()) - return InstrumentColdFunctionCoverageMode == - InstrColdFuncCovMode::Conservative; - return F.getEntryCount()->getCount() > ColdFuncCoverageMaxEntryCount; + if (auto EntryCount = F.getEntryCount()) + return EntryCount->getCount() > ColdFuncCoverageMaxEntryCount; + return InstrumentColdFunctionCoverageMode == + InstrColdFuncCovMode::Conservative; } return false; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits