llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-driver
Author: Lei Wang (wlei-llvm)
<details>
<summary>Changes</summary>
In https://github.com/llvm/llvm-project/pull/109837, it sets a global variable(`PGOInstrumentColdFunctionOnly`) in PassBuilderPipelines.cpp which introduced a data race detected by TSan. To fix this, I decouple the flag setting, the flags are now set separately(`instrument-cold-function-only-path` is required to be used with `--pgo-instrument-cold-function-only`).
---
Full diff: https://github.com/llvm/llvm-project/pull/114364.diff
3 Files Affected:
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+2)
- (modified) clang/test/Driver/fprofile-generate-cold-function-coverage.c (+1)
- (modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+10-5)
``````````diff
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)
``````````
</details>
https://github.com/llvm/llvm-project/pull/114364