Skip to content

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Oct 9, 2025

  • Make instruction arguments to the misexpect functions const and drop const for ArrayRef arguments.
  • Remove namespace llvm/misexpect surrounding all the code in .cpp file and instead use using namespace.
  • Change static helper functions to also use const references or pointers when possible.

@jurahul jurahul marked this pull request as ready for review October 9, 2025 22:36
@llvmbot
Copy link
Member

llvmbot commented Oct 9, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Rahul Joshi (jurahul)

Changes
  • Make instruction arguments to the misexpect functions const and drop const for ArrayRef arguments.
  • Remove namespace llvm/misexpect surrounding all the code in .cpp file and instead use using namespace.
  • Change static helper functions to also use const references or pointers when possible.

Full diff: https://github.com/llvm/llvm-project/pull/162695.diff

2 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Utils/MisExpect.h (+10-12)
  • (modified) llvm/lib/Transforms/Utils/MisExpect.cpp (+22-39)
diff --git a/llvm/include/llvm/Transforms/Utils/MisExpect.h b/llvm/include/llvm/Transforms/Utils/MisExpect.h
index e9fba47c97a4d..2d35d92d6b199 100644
--- a/llvm/include/llvm/Transforms/Utils/MisExpect.h
+++ b/llvm/include/llvm/Transforms/Utils/MisExpect.h
@@ -22,8 +22,7 @@
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
 
-namespace llvm {
-namespace misexpect {
+namespace llvm::misexpect {
 
 /// checkBackendInstrumentation - compares PGO counters to the thresholds used
 /// for llvm.expect and warns if the PGO counters are outside of the expected
@@ -34,8 +33,8 @@ namespace misexpect {
 ///
 /// \param I The Instruction being checked
 /// \param RealWeights A vector of profile weights for each target block
-void checkBackendInstrumentation(Instruction &I,
-                                 const llvm::ArrayRef<uint32_t> RealWeights);
+void checkBackendInstrumentation(const Instruction &I,
+                                 ArrayRef<uint32_t> RealWeights);
 
 /// checkFrontendInstrumentation - compares PGO counters to the thresholds used
 /// for llvm.expect and warns if the PGO counters are outside of the expected
@@ -48,8 +47,8 @@ void checkBackendInstrumentation(Instruction &I,
 /// \param I The Instruction being checked
 /// \param ExpectedWeights A vector of the expected weights for each target
 /// block, this determines the threshold values used when emitting diagnostics
-void checkFrontendInstrumentation(Instruction &I,
-                                  const ArrayRef<uint32_t> ExpectedWeights);
+void checkFrontendInstrumentation(const Instruction &I,
+                                  ArrayRef<uint32_t> ExpectedWeights);
 
 /// veryifyMisExpect - compares RealWeights to the thresholds used
 /// for llvm.expect and warns if the PGO counters are outside of the expected
@@ -58,8 +57,8 @@ void checkFrontendInstrumentation(Instruction &I,
 /// \param I The Instruction being checked
 /// \param RealWeights A vector of profile weights from the profile data
 /// \param ExpectedWeights A vector of the weights attatch by llvm.expect
-void verifyMisExpect(Instruction &I, ArrayRef<uint32_t> RealWeights,
-                     const ArrayRef<uint32_t> ExpectedWeights);
+void verifyMisExpect(const Instruction &I, ArrayRef<uint32_t> RealWeights,
+                     ArrayRef<uint32_t> ExpectedWeights);
 
 /// checkExpectAnnotations - compares PGO counters to the thresholds used
 /// for llvm.expect and warns if the PGO counters are outside of the expected
@@ -72,11 +71,10 @@ void verifyMisExpect(Instruction &I, ArrayRef<uint32_t> RealWeights,
 /// \param I The Instruction being checked
 /// \param ExistingWeights A vector of profile weights for each target block
 /// \param IsFrontend A boolean describing if this is Frontend instrumentation
-void checkExpectAnnotations(Instruction &I,
-                            const ArrayRef<uint32_t> ExistingWeights,
+void checkExpectAnnotations(const Instruction &I,
+                            ArrayRef<uint32_t> ExistingWeights,
                             bool IsFrontend);
 
-} // namespace misexpect
-} // namespace llvm
+} // namespace llvm::misexpect
 
 #endif
diff --git a/llvm/lib/Transforms/Utils/MisExpect.cpp b/llvm/lib/Transforms/Utils/MisExpect.cpp
index ca7e09da9737b..1585e9e509f89 100644
--- a/llvm/lib/Transforms/Utils/MisExpect.cpp
+++ b/llvm/lib/Transforms/Utils/MisExpect.cpp
@@ -48,8 +48,6 @@
 using namespace llvm;
 using namespace misexpect;
 
-namespace llvm {
-
 // Command line option to enable/disable the warning when profile data suggests
 // a mismatch with the use of the llvm.expect intrinsic
 static cl::opt<bool> PGOWarnMisExpect(
@@ -63,22 +61,18 @@ static cl::opt<uint32_t> MisExpectTolerance(
     cl::desc("Prevents emitting diagnostics when profile counts are "
              "within N% of the threshold.."));
 
-} // namespace llvm
-
-namespace {
-
-bool isMisExpectDiagEnabled(LLVMContext &Ctx) {
+static bool isMisExpectDiagEnabled(const LLVMContext &Ctx) {
   return PGOWarnMisExpect || Ctx.getMisExpectWarningRequested();
 }
 
-uint32_t getMisExpectTolerance(LLVMContext &Ctx) {
+static uint32_t getMisExpectTolerance(const LLVMContext &Ctx) {
   return std::max(static_cast<uint32_t>(MisExpectTolerance),
                   Ctx.getDiagnosticsMisExpectTolerance());
 }
 
-Instruction *getInstCondition(Instruction *I) {
+static const Instruction *getInstCondition(const Instruction *I) {
   assert(I != nullptr && "MisExpect target Instruction cannot be nullptr");
-  Instruction *Ret = nullptr;
+  const Instruction *Ret = nullptr;
   if (auto *B = dyn_cast<BranchInst>(I)) {
     Ret = dyn_cast<Instruction>(B->getCondition());
   }
@@ -97,8 +91,8 @@ Instruction *getInstCondition(Instruction *I) {
   return Ret ? Ret : I;
 }
 
-void emitMisexpectDiagnostic(Instruction *I, LLVMContext &Ctx,
-                             uint64_t ProfCount, uint64_t TotalCount) {
+static void emitMisexpectDiagnostic(const Instruction *I, LLVMContext &Ctx,
+                                    uint64_t ProfCount, uint64_t TotalCount) {
   double PercentageCorrect = (double)ProfCount / TotalCount;
   auto PerString =
       formatv("{0:P} ({1} / {2})", PercentageCorrect, ProfCount, TotalCount);
@@ -106,20 +100,16 @@ void emitMisexpectDiagnostic(Instruction *I, LLVMContext &Ctx,
       "Potential performance regression from use of the llvm.expect intrinsic: "
       "Annotation was correct on {0} of profiled executions.",
       PerString);
-  Instruction *Cond = getInstCondition(I);
+  const Instruction *Cond = getInstCondition(I);
   if (isMisExpectDiagEnabled(Ctx))
     Ctx.diagnose(DiagnosticInfoMisExpect(Cond, Twine(PerString)));
   OptimizationRemarkEmitter ORE(I->getParent()->getParent());
   ORE.emit(OptimizationRemark(DEBUG_TYPE, "misexpect", Cond) << RemStr.str());
 }
 
-} // namespace
-
-namespace llvm {
-namespace misexpect {
-
-void verifyMisExpect(Instruction &I, ArrayRef<uint32_t> RealWeights,
-                     ArrayRef<uint32_t> ExpectedWeights) {
+void misexpect::verifyMisExpect(const Instruction &I,
+                                ArrayRef<uint32_t> RealWeights,
+                                ArrayRef<uint32_t> ExpectedWeights) {
   // To determine if we emit a diagnostic, we need to compare the branch weights
   // from the profile to those added by the llvm.expect intrinsic.
   // So first, we extract the "likely" and "unlikely" weights from
@@ -128,15 +118,13 @@ void verifyMisExpect(Instruction &I, ArrayRef<uint32_t> RealWeights,
   uint64_t LikelyBranchWeight = 0,
            UnlikelyBranchWeight = std::numeric_limits<uint32_t>::max();
   size_t MaxIndex = 0;
-  for (size_t Idx = 0, End = ExpectedWeights.size(); Idx < End; Idx++) {
-    uint32_t V = ExpectedWeights[Idx];
+  for (const auto &[Idx, V] : enumerate(ExpectedWeights)) {
     if (LikelyBranchWeight < V) {
       LikelyBranchWeight = V;
       MaxIndex = Idx;
     }
-    if (UnlikelyBranchWeight > V) {
+    if (UnlikelyBranchWeight > V)
       UnlikelyBranchWeight = V;
-    }
   }
 
   const uint64_t ProfiledWeight = RealWeights[MaxIndex];
@@ -161,7 +149,7 @@ void verifyMisExpect(Instruction &I, ArrayRef<uint32_t> RealWeights,
   uint64_t ScaledThreshold = LikelyProbablilty.scale(RealWeightsTotal);
 
   // clamp tolerance range to [0, 100)
-  auto Tolerance = getMisExpectTolerance(I.getContext());
+  uint32_t Tolerance = getMisExpectTolerance(I.getContext());
   Tolerance = std::clamp(Tolerance, 0u, 99u);
 
   // Allow users to relax checking by N%  i.e., if they use a 5% tolerance,
@@ -175,8 +163,8 @@ void verifyMisExpect(Instruction &I, ArrayRef<uint32_t> RealWeights,
                             RealWeightsTotal);
 }
 
-void checkBackendInstrumentation(Instruction &I,
-                                 const ArrayRef<uint32_t> RealWeights) {
+void misexpect::checkBackendInstrumentation(const Instruction &I,
+                                            ArrayRef<uint32_t> RealWeights) {
   // Backend checking assumes any existing weight comes from an `llvm.expect`
   // intrinsic. However, SampleProfiling + ThinLTO add branch weights  multiple
   // times, leading to an invalid assumption in our checking. Backend checks
@@ -190,24 +178,19 @@ void checkBackendInstrumentation(Instruction &I,
   verifyMisExpect(I, RealWeights, ExpectedWeights);
 }
 
-void checkFrontendInstrumentation(Instruction &I,
-                                  const ArrayRef<uint32_t> ExpectedWeights) {
+void misexpect::checkFrontendInstrumentation(
+    const Instruction &I, ArrayRef<uint32_t> ExpectedWeights) {
   SmallVector<uint32_t> RealWeights;
   if (!extractBranchWeights(I, RealWeights))
     return;
   verifyMisExpect(I, RealWeights, ExpectedWeights);
 }
 
-void checkExpectAnnotations(Instruction &I,
-                            const ArrayRef<uint32_t> ExistingWeights,
-                            bool IsFrontend) {
-  if (IsFrontend) {
+void misexpect::checkExpectAnnotations(const Instruction &I,
+                                       ArrayRef<uint32_t> ExistingWeights,
+                                       bool IsFrontend) {
+  if (IsFrontend)
     checkFrontendInstrumentation(I, ExistingWeights);
-  } else {
+  else
     checkBackendInstrumentation(I, ExistingWeights);
-  }
 }
-
-} // namespace misexpect
-} // namespace llvm
-#undef DEBUG_TYPE

@jurahul jurahul merged commit 97031a6 into llvm:main Oct 10, 2025
13 checks passed
@jurahul jurahul deleted the nfc_codecleanup_misexpect branch October 10, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants