[clang] [llvm] [InstrPGO] Avoid using global variable to fix potential data race (PR #114364)

[Lei Wang via cfe-commits]

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 <wlei at 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)