https://github.com/wlei-llvm created https://github.com/llvm/llvm-project/pull/114364
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`). >From f912c30955541bd18f5fa56eff1c222672c0fa7f Mon Sep 17 00:00:00 2001 From: wlei <w...@fb.com> Date: Wed, 30 Oct 2024 22:21:24 -0700 Subject: [PATCH] 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) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
