https://github.com/wlei-llvm updated https://github.com/llvm/llvm-project/pull/114364
>From a7b444bd75d6f83ed0f5692783990a59f36e8459 Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Thu, 31 Oct 2024 09:58:27 -0700 Subject: [PATCH 1/4] Reapply "[InstrPGO] Support cold function coverage instrumentation (#109837)" This reverts commit d924a9ba03a05b417676e84f6c81aac44f907f71. --- clang/include/clang/Driver/Options.td | 6 ++++ clang/lib/Driver/ToolChain.cpp | 4 ++- clang/lib/Driver/ToolChains/Clang.cpp | 20 +++++++++++ .../test/CodeGen/pgo-cold-function-coverage.c | 19 ++++++++++ ...fprofile-generate-cold-function-coverage.c | 8 +++++ llvm/lib/Passes/PassBuilderPipelines.cpp | 17 ++++++++- .../Instrumentation/PGOInstrumentation.cpp | 19 ++++++++++ .../PGOProfile/instr-gen-cold-function.ll | 35 +++++++++++++++++++ 8 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c create mode 100644 clang/test/Driver/fprofile-generate-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 c8bc2fe377b8ec..2814d2b1bf3733 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1786,6 +1786,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 collect coverage info for 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 collect coverage info for 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 6d3ede40691093..bdf3da0c96adca 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -899,7 +899,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 04b3832327a99c..4c6f508f1f24a6 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -632,6 +632,26 @@ 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"); + // FIXME: Idealy the file path should be passed through + // `-fprofile-instrument-path=`(InstrProfileOutput), however, this field is + // shared with other profile use path(see PGOOptions), we need to refactor + // PGOOptions to make it work. + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back(Args.MakeArgString( + Twine("--instrument-cold-function-only-path=") + Path)); + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back("--pgo-function-entry-coverage"); + } + Arg *PGOGenArg = nullptr; if (PGOGenerateArg) { assert(!CSPGOGenerateArg); 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..fd1e1e7e14cda5 --- /dev/null +++ b/clang/test/CodeGen/pgo-cold-function-coverage.c @@ -0,0 +1,19 @@ +// Test -fprofile-generate-cold-function-coverage + +// RUN: rm -rf %t && 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: 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..9b2f46423f34b1 --- /dev/null +++ b/clang/test/Driver/fprofile-generate-cold-function-coverage.c @@ -0,0 +1,8 @@ +// RUN: %clang -### -c -fprofile-generate-cold-function-coverage %s 2>&1 | FileCheck %s +// CHECK: "--instrument-cold-function-only-path=default_%m.profraw" +// 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-EQ +// CHECK-EQ: "--instrument-cold-function-only-path=dir{{/|\\\\}}default_%m.profraw" diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index 6478667e22b851..c391853c8d0c2b 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -296,7 +296,12 @@ 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> InstrumentColdFuncOnlyPath( + "instrument-cold-function-only-path", cl::init(""), + cl::desc("File path for cold function only instrumentation"), cl::Hidden); + extern cl::opt<std::string> UseCtxProfile; +extern cl::opt<bool> PGOInstrumentColdFunctionOnly; namespace llvm { extern cl::opt<bool> EnableMemProfContextDisambiguation; @@ -1183,8 +1188,13 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, const bool IsCtxProfUse = !UseCtxProfile.empty() && Phase == ThinOrFullLTOPhase::ThinLTOPreLink; + // Enable cold function coverage instrumentation if + // InstrumentColdFuncOnlyPath is provided. + const bool IsColdFuncOnlyInstrGen = PGOInstrumentColdFunctionOnly = + IsPGOPreLink && !InstrumentColdFuncOnlyPath.empty(); + if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen || - IsCtxProfUse) + IsCtxProfUse || IsColdFuncOnlyInstrGen) addPreInlinerPasses(MPM, Level, Phase); // Add all the requested passes for instrumentation PGO, if requested. @@ -1206,6 +1216,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, return MPM; addPostPGOLoopRotation(MPM, Level); MPM.addPass(PGOCtxProfLoweringPass()); + } else if (IsColdFuncOnlyInstrGen) { + addPGOInstrPasses( + MPM, Level, /* RunProfileGen */ true, /* IsCS */ false, + /* AtomicCounterUpdate */ false, InstrumentColdFuncOnlyPath, + /* ProfileRemappingFile */ "", IntrusiveRefCntPtr<vfs::FileSystem>()); } if (IsPGOInstrGen || IsPGOInstrUse || IsCtxProfGen) diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index bceb6135cc1f92..4d8141431a0c19 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -319,6 +319,20 @@ static cl::opt<unsigned> PGOFunctionCriticalEdgeThreshold( cl::desc("Do not instrument functions with the number of critical edges " " greater than this threshold.")); +static cl::opt<uint64_t> PGOColdInstrumentEntryThreshold( + "pgo-cold-instrument-entry-threshold", cl::init(0), cl::Hidden, + cl::desc("For cold function instrumentation, skip instrumenting functions " + "whose entry count is above the given value.")); + +static cl::opt<bool> PGOTreatUnknownAsCold( + "pgo-treat-unknown-as-cold", cl::init(false), cl::Hidden, + cl::desc("For cold function instrumentation, treat count unknown(e.g. " + "unprofiled) functions as cold.")); + +cl::opt<bool> PGOInstrumentColdFunctionOnly( + "pgo-instrument-cold-function-only", cl::init(false), cl::Hidden, + cl::desc("Enable cold function only instrumentation.")); + extern cl::opt<unsigned> MaxNumVTableAnnotations; namespace llvm { @@ -1897,6 +1911,11 @@ static bool skipPGOGen(const Function &F) { return true; if (F.getInstructionCount() < PGOFunctionSizeThreshold) return true; + if (PGOInstrumentColdFunctionOnly) { + if (auto EntryCount = F.getEntryCount()) + return EntryCount->getCount() > PGOColdInstrumentEntryThreshold; + return !PGOTreatUnknownAsCold; + } 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..ab8cf8c010812b --- /dev/null +++ b/llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll @@ -0,0 +1,35 @@ +; RUN: opt < %s --passes=pgo-instr-gen -pgo-instrument-cold-function-only -pgo-function-entry-coverage -S | FileCheck --check-prefixes=COLD %s +; 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 +; 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 + +; 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) + +; UNKNOWN-FUNC: call void @llvm.instrprof.cover(ptr @__profn_bar, i64 [[#]], i32 1, i32 0) +; UNKNOWN-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 +} + +define i32 @main() !prof !1 { +entry: + ret i32 0 +} + +!0 = !{!"function_entry_count", i64 0} +!1 = !{!"function_entry_count", i64 1} >From d9828e83d760158461baac2b3fd204465de52570 Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Thu, 31 Oct 2024 09:59:15 -0700 Subject: [PATCH 2/4] Reapply "specify clang --target to fix breakage on AIX (#114127)" This reverts commit 06e28ed84f3f0a05b493699965e18e7a5aeaccd2. --- clang/test/CodeGen/pgo-cold-function-coverage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c b/clang/test/CodeGen/pgo-cold-function-coverage.c index fd1e1e7e14cda5..3003cdc3e15e02 100644 --- a/clang/test/CodeGen/pgo-cold-function-coverage.c +++ b/clang/test/CodeGen/pgo-cold-function-coverage.c @@ -1,7 +1,7 @@ // Test -fprofile-generate-cold-function-coverage // RUN: rm -rf %t && 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 +// RUN: %clang --target=x86_64 -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" >From 248273230702f9c16f6334b2143334787db6f862 Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Wed, 30 Oct 2024 22:21:24 -0700 Subject: [PATCH 3/4] fix datarace --- clang/lib/Driver/ToolChains/Clang.cpp | 2 ++ .../fprofile-generate-cold-function-coverage.c | 1 + llvm/lib/Passes/PassBuilderPipelines.cpp | 15 ++++++++++----- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 4c6f508f1f24a6..04ae99d417d4f7 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -649,6 +649,8 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C, CmdArgs.push_back(Args.MakeArgString( Twine("--instrument-cold-function-only-path=") + Path)); CmdArgs.push_back("-mllvm"); + CmdArgs.push_back("--pgo-instrument-cold-function-only"); + CmdArgs.push_back("-mllvm"); CmdArgs.push_back("--pgo-function-entry-coverage"); } diff --git a/clang/test/Driver/fprofile-generate-cold-function-coverage.c b/clang/test/Driver/fprofile-generate-cold-function-coverage.c index 9b2f46423f34b1..a8ca69cf224cd0 100644 --- a/clang/test/Driver/fprofile-generate-cold-function-coverage.c +++ b/clang/test/Driver/fprofile-generate-cold-function-coverage.c @@ -1,6 +1,7 @@ // RUN: %clang -### -c -fprofile-generate-cold-function-coverage %s 2>&1 | FileCheck %s // CHECK: "--instrument-cold-function-only-path=default_%m.profraw" // CHECK: "--pgo-function-entry-coverage" +// CHECK: "--pgo-instrument-cold-function-only" // CHECK-NOT: "-fprofile-instrument" // CHECK-NOT: "-fprofile-instrument-path= diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index c391853c8d0c2b..d5ef7c654c35cb 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -298,7 +298,9 @@ static cl::opt<bool> UseLoopVersioningLICM( static cl::opt<std::string> InstrumentColdFuncOnlyPath( "instrument-cold-function-only-path", cl::init(""), - cl::desc("File path for cold function only instrumentation"), cl::Hidden); + cl::desc("File path for cold function only instrumentation(requires use " + "with --pgo-instrument-cold-function-only)"), + cl::Hidden); extern cl::opt<std::string> UseCtxProfile; extern cl::opt<bool> PGOInstrumentColdFunctionOnly; @@ -1188,10 +1190,13 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, const bool IsCtxProfUse = !UseCtxProfile.empty() && Phase == ThinOrFullLTOPhase::ThinLTOPreLink; - // Enable cold function coverage instrumentation if - // InstrumentColdFuncOnlyPath is provided. - const bool IsColdFuncOnlyInstrGen = PGOInstrumentColdFunctionOnly = - IsPGOPreLink && !InstrumentColdFuncOnlyPath.empty(); + assert( + (InstrumentColdFuncOnlyPath.empty() || PGOInstrumentColdFunctionOnly) && + "--instrument-cold-function-only-path is provided but " + "--pgo-instrument-cold-function-only is not enabled"); + const bool IsColdFuncOnlyInstrGen = PGOInstrumentColdFunctionOnly && + IsPGOPreLink && + !InstrumentColdFuncOnlyPath.empty(); if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen || IsCtxProfUse || IsColdFuncOnlyInstrGen) >From 2b5520ca2dd33e2daaa8782eb63dc119c2235167 Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Thu, 31 Oct 2024 14:34:35 -0700 Subject: [PATCH 4/4] fix test failure --- clang/test/Driver/fprofile-generate-cold-function-coverage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Driver/fprofile-generate-cold-function-coverage.c b/clang/test/Driver/fprofile-generate-cold-function-coverage.c index a8ca69cf224cd0..135acf2e736f79 100644 --- a/clang/test/Driver/fprofile-generate-cold-function-coverage.c +++ b/clang/test/Driver/fprofile-generate-cold-function-coverage.c @@ -1,7 +1,7 @@ // RUN: %clang -### -c -fprofile-generate-cold-function-coverage %s 2>&1 | FileCheck %s // CHECK: "--instrument-cold-function-only-path=default_%m.profraw" -// CHECK: "--pgo-function-entry-coverage" // CHECK: "--pgo-instrument-cold-function-only" +// CHECK: "--pgo-function-entry-coverage" // CHECK-NOT: "-fprofile-instrument" // CHECK-NOT: "-fprofile-instrument-path= _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits