[PGO] Add ProfileInjector and ProfileVerifier passes #147388

mtrofin · 2025-07-07T20:26:11Z

Adding 2 passes, one to inject MD_prof and one to check its presence. A subsequent patch will add these (similar to debugify) to opt (and, eventually, a variant of this, to llc)

Tracking issue: #147390

mtrofin · 2025-07-07T20:26:28Z

[PGO] Drive profile validator from opt #147418
[PGO] Add ProfileInjector and ProfileVerifier passes #147388 👈 (View in Graphite)
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · 2025-07-07T20:31:27Z

@llvm/pr-subscribers-pgo

Author: Mircea Trofin (mtrofin)

Changes

Adding 2 passes, one to inject MD_prof and one to check its presence. A subsequent patch will add these (similar to debugify) to opt (and, eventually, a variant of this, to llc)

Tracking issue: #147390

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

8 Files Affected:

(added) llvm/include/llvm/Transforms/Utils/ProfileVerify.h (+36)
(modified) llvm/lib/Passes/PassBuilder.cpp (+1)
(modified) llvm/lib/Passes/PassRegistry.def (+2)
(modified) llvm/lib/Transforms/Utils/CMakeLists.txt (+1)
(added) llvm/lib/Transforms/Utils/ProfileVerify.cpp (+113)
(added) llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll (+19)
(added) llvm/test/Transforms/PGOProfile/prof-verify-existing.ll (+21)
(added) llvm/test/Transforms/PGOProfile/prof-verify.ll (+19)

diff --git a/llvm/include/llvm/Transforms/Utils/ProfileVerify.h b/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
new file mode 100644
index 0000000000000..88942a73474d4
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
@@ -0,0 +1,36 @@
+//===- ProfileVerify.h - Verify profile info for testing ----0-----*-C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Inject profile information, as part of tests, to verify passes don't
+// accidentally drop it.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TRANSFORMS_UTILS_PROFILEVERIFY_H
+#define LLVM_TRANSFORMS_UTILS_PROFILEVERIFY_H
+
+#include "llvm/IR/Analysis.h"
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+/// Inject MD_prof metadata where it's missing. Used for testing that passes
+/// don't accidentally drop this metadata.
+class ProfileInjectorPass : public PassInfoMixin<ProfileInjectorPass> {
+public:
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
+};
+
+/// Checks that MD_prof is present on every instruction that supports it. Used
+/// in conjunction with the ProfileInjectorPass. MD_prof "unknown" is considered
+/// valid (i.e. !{!"unknown"})
+class ProfileVerifierPass : public PassInfoMixin<ProfileVerifierPass> {
+public:
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
+};
+
+} // namespace llvm
+#endif
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 874fce05841e2..70c0e999cbb95 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -357,6 +357,7 @@
 #include "llvm/Transforms/Utils/MoveAutoInit.h"
 #include "llvm/Transforms/Utils/NameAnonGlobals.h"
 #include "llvm/Transforms/Utils/PredicateInfo.h"
+#include "llvm/Transforms/Utils/ProfileVerify.h"
 #include "llvm/Transforms/Utils/RelLookupTableConverter.h"
 #include "llvm/Transforms/Utils/StripGCRelocates.h"
 #include "llvm/Transforms/Utils/StripNonLineTableDebugInfo.h"
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index dd3dab3425975..dfe233f15a149 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -517,6 +517,8 @@ FUNCTION_PASS("print<regions>", RegionInfoPrinterPass(errs()))
 FUNCTION_PASS("print<scalar-evolution>", ScalarEvolutionPrinterPass(errs()))
 FUNCTION_PASS("print<stack-safety-local>", StackSafetyPrinterPass(errs()))
 FUNCTION_PASS("print<uniformity>", UniformityInfoPrinterPass(errs()))
+FUNCTION_PASS("prof-inject", ProfileInjectorPass())
+FUNCTION_PASS("prof-verify", ProfileVerifierPass())
 FUNCTION_PASS("reassociate", ReassociatePass())
 FUNCTION_PASS("redundant-dbg-inst-elim", RedundantDbgInstEliminationPass())
 FUNCTION_PASS("reg2mem", RegToMemPass())
diff --git a/llvm/lib/Transforms/Utils/CMakeLists.txt b/llvm/lib/Transforms/Utils/CMakeLists.txt
index 78cad0d253be8..c0bd6d647aad4 100644
--- a/llvm/lib/Transforms/Utils/CMakeLists.txt
+++ b/llvm/lib/Transforms/Utils/CMakeLists.txt
@@ -67,6 +67,7 @@ add_llvm_component_library(LLVMTransformUtils
   MoveAutoInit.cpp
   NameAnonGlobals.cpp
   PredicateInfo.cpp
+  ProfileVerify.cpp
   PromoteMemoryToRegister.cpp
   RelLookupTableConverter.cpp
   ScalarEvolutionExpander.cpp
diff --git a/llvm/lib/Transforms/Utils/ProfileVerify.cpp b/llvm/lib/Transforms/Utils/ProfileVerify.cpp
new file mode 100644
index 0000000000000..e0d220e998548
--- /dev/null
+++ b/llvm/lib/Transforms/Utils/ProfileVerify.cpp
@@ -0,0 +1,113 @@
+//===- ProfileVerify.cpp - Verify profile info for testing ----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Utils/ProfileVerify.h"
+#include "llvm/ADT/DynamicAPInt.h"
+#include "llvm/ADT/PostOrderIterator.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Analysis/BranchProbabilityInfo.h"
+#include "llvm/Analysis/LoopInfo.h"
+#include "llvm/IR/Analysis.h"
+#include "llvm/IR/Dominators.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/ProfDataUtils.h"
+#include "llvm/Support/BranchProbability.h"
+
+using namespace llvm;
+namespace {
+class ProfileInjector {
+  Function &F;
+  FunctionAnalysisManager &FAM;
+
+public:
+  static bool supportsBranchWeights(const Instruction &I) {
+    return isa<BranchInst>(&I) ||
+
+           isa<SwitchInst>(&I) ||
+
+           isa<IndirectBrInst>(&I) || isa<SelectInst>(&I) ||
+           isa<CallBrInst>(&I);
+  }
+
+  ProfileInjector(Function &F, FunctionAnalysisManager &FAM) : F(F), FAM(FAM) {}
+  bool inject();
+};
+} // namespace
+
+bool ProfileInjector::inject() {
+  auto &BPI = FAM.getResult<BranchProbabilityAnalysis>(F);
+
+  for (auto &BB : F) {
+    if (succ_size(&BB) <= 1)
+      continue;
+    auto *Term = BB.getTerminator();
+    assert(Term);
+    if (Term->getMetadata(LLVMContext::MD_prof) ||
+        !supportsBranchWeights(*Term))
+      continue;
+    SmallVector<BranchProbability> Probs;
+    Probs.reserve(Term->getNumSuccessors());
+    for (auto I = 0U, E = Term->getNumSuccessors(); I < E; ++I)
+      Probs.emplace_back(BPI.getEdgeProbability(&BB, Term->getSuccessor(I)));
+
+    const auto *FirstZeroDenominator =
+        find_if(Probs, [](const BranchProbability &P) {
+          return P.getDenominator() == 0;
+        });
+    assert(FirstZeroDenominator == Probs.end());
+    const auto *FirstNonzeroNumerator =
+        find_if(Probs, [](const BranchProbability &P) {
+          return P.getNumerator() != 0;
+        });
+    assert(FirstNonzeroNumerator != Probs.end());
+    DynamicAPInt LCM(Probs[0].getDenominator());
+    DynamicAPInt GCD(FirstNonzeroNumerator->getNumerator());
+    for (const auto &Prob : drop_begin(Probs)) {
+      if (!Prob.getNumerator())
+        continue;
+      LCM = llvm::lcm(LCM, DynamicAPInt(Prob.getDenominator()));
+      GCD = llvm::lcm(GCD, DynamicAPInt(Prob.getNumerator()));
+    }
+    SmallVector<uint32_t> Weights;
+    Weights.reserve(Term->getNumSuccessors());
+    for (const auto &Prob : Probs) {
+      auto W = Prob.getNumerator() * LCM / GCD;
+      Weights.emplace_back(static_cast<int32_t>((int64_t)W));
+    }
+    setBranchWeights(*Term, Weights, false);
+  }
+  return true;
+}
+
+PreservedAnalyses ProfileInjectorPass::run(Function &F,
+                                           FunctionAnalysisManager &FAM) {
+  ProfileInjector PI(F, FAM);
+  if (!PI.inject())
+    return PreservedAnalyses::all();
+
+  return PreservedAnalyses::none();
+}
+
+PreservedAnalyses ProfileVerifierPass::run(Function &F,
+                                           FunctionAnalysisManager &FAM) {
+  bool Changed = false;
+  for (auto &BB : F)
+    if (succ_size(&BB) >= 2)
+      if (auto *Term = BB.getTerminator())
+        if (ProfileInjector::supportsBranchWeights(*Term)) {
+          if (!Term->getMetadata(LLVMContext::MD_prof)) {
+            F.getContext().emitError("Profile verification failed");
+          } else {
+            Changed = true;
+          }
+        }
+
+  return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
+}
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll b/llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll
new file mode 100644
index 0000000000000..19d60d03bd873
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll
@@ -0,0 +1,19 @@
+; Test that prof-inject only injects missing metadata
+
+; RUN: opt -passes=prof-inject %s -S -o - | FileCheck %s
+
+define void @foo(i32 %i) {
+  %c = icmp eq i32 %i, 0
+  br i1 %c, label %yes, label %no, !prof !0
+yes:
+  br i1 %c, label %yes2, label %no
+yes2:
+  ret void
+no:
+  ret void
+}
+
+!0 = !{!"branch_weights", i32 1, i32 2}
+; CHECK: br i1 %c, label %yes, label %no, !prof !0
+; CHECK: !0 = !{!"branch_weights", i32 1, i32 2}
+; CHECK: !1 = !{!"branch_weights", i32 429496729, i32 715827882}
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify-existing.ll b/llvm/test/Transforms/PGOProfile/prof-verify-existing.ll
new file mode 100644
index 0000000000000..d9cc038bc3204
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/prof-verify-existing.ll
@@ -0,0 +1,21 @@
+; Test that prof-verify does not modify existing metadata (incl. "unknown")
+
+; RUN: opt -passes=prof-inject %s -S -o - | FileCheck %s
+; RUN: opt -passes=prof-verify %s -S --disable-output
+
+define void @foo(i32 %i) {
+  %c = icmp eq i32 %i, 0
+  br i1 %c, label %yes, label %no, !prof !0
+yes:
+  br i1 %c, label %yes2, label %no, !prof !1
+yes2:
+  ret void
+no:
+  ret void
+}
+
+!0 = !{!"branch_weights", i32 1, i32 2}
+!1 = !{!"unknown"}
+; CHECK: br i1 %c, label %yes, label %no, !prof !0
+; CHECK: !0 = !{!"branch_weights", i32 1, i32 2}
+; CHECK: !1 = !{!"unknown"}
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify.ll b/llvm/test/Transforms/PGOProfile/prof-verify.ll
new file mode 100644
index 0000000000000..f4d2b2e51dc59
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/prof-verify.ll
@@ -0,0 +1,19 @@
+; Test prof-inject and prof-verify
+
+; RUN: opt -passes=prof-inject %s -S -o - | FileCheck %s --check-prefix=INJECT
+; RUN: not opt -passes=prof-verify %s -S -o - 2>&1 | FileCheck %s --check-prefix=VERIFY
+; RUN: opt -passes=prof-inject,prof-verify %s --disable-output
+
+define void @foo(i32 %i) {
+  %c = icmp eq i32 %i, 0
+  br i1 %c, label %yes, label %no
+yes:
+  ret void
+no:
+  ret void
+}
+
+; INJECT: br i1 %c, label %yes, label %no, !prof !0
+; INJECT: !0 = !{!"branch_weights", i32 429496729, i32 715827882}
+
+; VERIFY: Profile verification failed
\ No newline at end of file

llvmbot · 2025-07-07T20:31:27Z

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

Adding 2 passes, one to inject MD_prof and one to check its presence. A subsequent patch will add these (similar to debugify) to opt (and, eventually, a variant of this, to llc)

Tracking issue: #147390

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

8 Files Affected:

(added) llvm/include/llvm/Transforms/Utils/ProfileVerify.h (+36)
(modified) llvm/lib/Passes/PassBuilder.cpp (+1)
(modified) llvm/lib/Passes/PassRegistry.def (+2)
(modified) llvm/lib/Transforms/Utils/CMakeLists.txt (+1)
(added) llvm/lib/Transforms/Utils/ProfileVerify.cpp (+113)
(added) llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll (+19)
(added) llvm/test/Transforms/PGOProfile/prof-verify-existing.ll (+21)
(added) llvm/test/Transforms/PGOProfile/prof-verify.ll (+19)

diff --git a/llvm/include/llvm/Transforms/Utils/ProfileVerify.h b/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
new file mode 100644
index 0000000000000..88942a73474d4
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
@@ -0,0 +1,36 @@
+//===- ProfileVerify.h - Verify profile info for testing ----0-----*-C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Inject profile information, as part of tests, to verify passes don't
+// accidentally drop it.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TRANSFORMS_UTILS_PROFILEVERIFY_H
+#define LLVM_TRANSFORMS_UTILS_PROFILEVERIFY_H
+
+#include "llvm/IR/Analysis.h"
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+/// Inject MD_prof metadata where it's missing. Used for testing that passes
+/// don't accidentally drop this metadata.
+class ProfileInjectorPass : public PassInfoMixin<ProfileInjectorPass> {
+public:
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
+};
+
+/// Checks that MD_prof is present on every instruction that supports it. Used
+/// in conjunction with the ProfileInjectorPass. MD_prof "unknown" is considered
+/// valid (i.e. !{!"unknown"})
+class ProfileVerifierPass : public PassInfoMixin<ProfileVerifierPass> {
+public:
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
+};
+
+} // namespace llvm
+#endif
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 874fce05841e2..70c0e999cbb95 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -357,6 +357,7 @@
 #include "llvm/Transforms/Utils/MoveAutoInit.h"
 #include "llvm/Transforms/Utils/NameAnonGlobals.h"
 #include "llvm/Transforms/Utils/PredicateInfo.h"
+#include "llvm/Transforms/Utils/ProfileVerify.h"
 #include "llvm/Transforms/Utils/RelLookupTableConverter.h"
 #include "llvm/Transforms/Utils/StripGCRelocates.h"
 #include "llvm/Transforms/Utils/StripNonLineTableDebugInfo.h"
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index dd3dab3425975..dfe233f15a149 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -517,6 +517,8 @@ FUNCTION_PASS("print<regions>", RegionInfoPrinterPass(errs()))
 FUNCTION_PASS("print<scalar-evolution>", ScalarEvolutionPrinterPass(errs()))
 FUNCTION_PASS("print<stack-safety-local>", StackSafetyPrinterPass(errs()))
 FUNCTION_PASS("print<uniformity>", UniformityInfoPrinterPass(errs()))
+FUNCTION_PASS("prof-inject", ProfileInjectorPass())
+FUNCTION_PASS("prof-verify", ProfileVerifierPass())
 FUNCTION_PASS("reassociate", ReassociatePass())
 FUNCTION_PASS("redundant-dbg-inst-elim", RedundantDbgInstEliminationPass())
 FUNCTION_PASS("reg2mem", RegToMemPass())
diff --git a/llvm/lib/Transforms/Utils/CMakeLists.txt b/llvm/lib/Transforms/Utils/CMakeLists.txt
index 78cad0d253be8..c0bd6d647aad4 100644
--- a/llvm/lib/Transforms/Utils/CMakeLists.txt
+++ b/llvm/lib/Transforms/Utils/CMakeLists.txt
@@ -67,6 +67,7 @@ add_llvm_component_library(LLVMTransformUtils
   MoveAutoInit.cpp
   NameAnonGlobals.cpp
   PredicateInfo.cpp
+  ProfileVerify.cpp
   PromoteMemoryToRegister.cpp
   RelLookupTableConverter.cpp
   ScalarEvolutionExpander.cpp
diff --git a/llvm/lib/Transforms/Utils/ProfileVerify.cpp b/llvm/lib/Transforms/Utils/ProfileVerify.cpp
new file mode 100644
index 0000000000000..e0d220e998548
--- /dev/null
+++ b/llvm/lib/Transforms/Utils/ProfileVerify.cpp
@@ -0,0 +1,113 @@
+//===- ProfileVerify.cpp - Verify profile info for testing ----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Utils/ProfileVerify.h"
+#include "llvm/ADT/DynamicAPInt.h"
+#include "llvm/ADT/PostOrderIterator.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Analysis/BranchProbabilityInfo.h"
+#include "llvm/Analysis/LoopInfo.h"
+#include "llvm/IR/Analysis.h"
+#include "llvm/IR/Dominators.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/ProfDataUtils.h"
+#include "llvm/Support/BranchProbability.h"
+
+using namespace llvm;
+namespace {
+class ProfileInjector {
+  Function &F;
+  FunctionAnalysisManager &FAM;
+
+public:
+  static bool supportsBranchWeights(const Instruction &I) {
+    return isa<BranchInst>(&I) ||
+
+           isa<SwitchInst>(&I) ||
+
+           isa<IndirectBrInst>(&I) || isa<SelectInst>(&I) ||
+           isa<CallBrInst>(&I);
+  }
+
+  ProfileInjector(Function &F, FunctionAnalysisManager &FAM) : F(F), FAM(FAM) {}
+  bool inject();
+};
+} // namespace
+
+bool ProfileInjector::inject() {
+  auto &BPI = FAM.getResult<BranchProbabilityAnalysis>(F);
+
+  for (auto &BB : F) {
+    if (succ_size(&BB) <= 1)
+      continue;
+    auto *Term = BB.getTerminator();
+    assert(Term);
+    if (Term->getMetadata(LLVMContext::MD_prof) ||
+        !supportsBranchWeights(*Term))
+      continue;
+    SmallVector<BranchProbability> Probs;
+    Probs.reserve(Term->getNumSuccessors());
+    for (auto I = 0U, E = Term->getNumSuccessors(); I < E; ++I)
+      Probs.emplace_back(BPI.getEdgeProbability(&BB, Term->getSuccessor(I)));
+
+    const auto *FirstZeroDenominator =
+        find_if(Probs, [](const BranchProbability &P) {
+          return P.getDenominator() == 0;
+        });
+    assert(FirstZeroDenominator == Probs.end());
+    const auto *FirstNonzeroNumerator =
+        find_if(Probs, [](const BranchProbability &P) {
+          return P.getNumerator() != 0;
+        });
+    assert(FirstNonzeroNumerator != Probs.end());
+    DynamicAPInt LCM(Probs[0].getDenominator());
+    DynamicAPInt GCD(FirstNonzeroNumerator->getNumerator());
+    for (const auto &Prob : drop_begin(Probs)) {
+      if (!Prob.getNumerator())
+        continue;
+      LCM = llvm::lcm(LCM, DynamicAPInt(Prob.getDenominator()));
+      GCD = llvm::lcm(GCD, DynamicAPInt(Prob.getNumerator()));
+    }
+    SmallVector<uint32_t> Weights;
+    Weights.reserve(Term->getNumSuccessors());
+    for (const auto &Prob : Probs) {
+      auto W = Prob.getNumerator() * LCM / GCD;
+      Weights.emplace_back(static_cast<int32_t>((int64_t)W));
+    }
+    setBranchWeights(*Term, Weights, false);
+  }
+  return true;
+}
+
+PreservedAnalyses ProfileInjectorPass::run(Function &F,
+                                           FunctionAnalysisManager &FAM) {
+  ProfileInjector PI(F, FAM);
+  if (!PI.inject())
+    return PreservedAnalyses::all();
+
+  return PreservedAnalyses::none();
+}
+
+PreservedAnalyses ProfileVerifierPass::run(Function &F,
+                                           FunctionAnalysisManager &FAM) {
+  bool Changed = false;
+  for (auto &BB : F)
+    if (succ_size(&BB) >= 2)
+      if (auto *Term = BB.getTerminator())
+        if (ProfileInjector::supportsBranchWeights(*Term)) {
+          if (!Term->getMetadata(LLVMContext::MD_prof)) {
+            F.getContext().emitError("Profile verification failed");
+          } else {
+            Changed = true;
+          }
+        }
+
+  return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
+}
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll b/llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll
new file mode 100644
index 0000000000000..19d60d03bd873
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll
@@ -0,0 +1,19 @@
+; Test that prof-inject only injects missing metadata
+
+; RUN: opt -passes=prof-inject %s -S -o - | FileCheck %s
+
+define void @foo(i32 %i) {
+  %c = icmp eq i32 %i, 0
+  br i1 %c, label %yes, label %no, !prof !0
+yes:
+  br i1 %c, label %yes2, label %no
+yes2:
+  ret void
+no:
+  ret void
+}
+
+!0 = !{!"branch_weights", i32 1, i32 2}
+; CHECK: br i1 %c, label %yes, label %no, !prof !0
+; CHECK: !0 = !{!"branch_weights", i32 1, i32 2}
+; CHECK: !1 = !{!"branch_weights", i32 429496729, i32 715827882}
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify-existing.ll b/llvm/test/Transforms/PGOProfile/prof-verify-existing.ll
new file mode 100644
index 0000000000000..d9cc038bc3204
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/prof-verify-existing.ll
@@ -0,0 +1,21 @@
+; Test that prof-verify does not modify existing metadata (incl. "unknown")
+
+; RUN: opt -passes=prof-inject %s -S -o - | FileCheck %s
+; RUN: opt -passes=prof-verify %s -S --disable-output
+
+define void @foo(i32 %i) {
+  %c = icmp eq i32 %i, 0
+  br i1 %c, label %yes, label %no, !prof !0
+yes:
+  br i1 %c, label %yes2, label %no, !prof !1
+yes2:
+  ret void
+no:
+  ret void
+}
+
+!0 = !{!"branch_weights", i32 1, i32 2}
+!1 = !{!"unknown"}
+; CHECK: br i1 %c, label %yes, label %no, !prof !0
+; CHECK: !0 = !{!"branch_weights", i32 1, i32 2}
+; CHECK: !1 = !{!"unknown"}
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify.ll b/llvm/test/Transforms/PGOProfile/prof-verify.ll
new file mode 100644
index 0000000000000..f4d2b2e51dc59
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/prof-verify.ll
@@ -0,0 +1,19 @@
+; Test prof-inject and prof-verify
+
+; RUN: opt -passes=prof-inject %s -S -o - | FileCheck %s --check-prefix=INJECT
+; RUN: not opt -passes=prof-verify %s -S -o - 2>&1 | FileCheck %s --check-prefix=VERIFY
+; RUN: opt -passes=prof-inject,prof-verify %s --disable-output
+
+define void @foo(i32 %i) {
+  %c = icmp eq i32 %i, 0
+  br i1 %c, label %yes, label %no
+yes:
+  ret void
+no:
+  ret void
+}
+
+; INJECT: br i1 %c, label %yes, label %no, !prof !0
+; INJECT: !0 = !{!"branch_weights", i32 429496729, i32 715827882}
+
+; VERIFY: Profile verification failed
\ No newline at end of file

david-xl · 2025-07-07T22:49:58Z

llvm/lib/Transforms/Utils/ProfileVerify.cpp

+};
+} // namespace
+
+bool ProfileInjector::inject() {


Can SamplePGO's weight propagation be reused here?

(capturing offline chat) this would be e.g. profi, and yes, we could use it in a number of ways:

instead of the perhaps more naive BPI estimator currently used (implicitly) here

as the way the BPI estimator handles MD_prof unknown cases (I think these 2 items basically amount to the same thing - improving the BPI estimator)

as a numerical validation heuristic (part "2" in the RFC)

llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll

david-xl · 2025-07-07T22:59:16Z

llvm/test/Transforms/PGOProfile/prof-verify-existing.ll

@@ -0,0 +1,21 @@
+; Test that prof-verify does not modify existing metadata (incl. "unknown")


Whether overriding unknown MD_prof should probably be guarded by a flag. There are cases the pass can not do a sophisticated weight propagation to infer weight for new branches, but the prof injection pass can.

The expectation is that unknown is inserted by the developer of a pass, so the injector shouldn't override it, since the injector is only here for testing purposes - i.e. its job is to make metadata "be there" for .ll tests that don't originally have it.

jdenny-ornl · 2025-07-16T22:58:10Z

llvm/lib/Transforms/Utils/ProfileVerify.cpp

+    const auto *FirstZeroDenominator =
+        find_if(Probs, [](const BranchProbability &P) {
+          return P.getDenominator() == 0;
+        });


Guard with NDEBUG?

ah, because only used in assert - done (something similar)

You cast FirstZeroDenominator to void, which would suppress any unused warning. I was thinking of avoiding useless code execution... unless we can trust the compiler will optimize it out.

The whole expression is const, so it should be optimizable out. I'd prefer relying on that, and fixing the DCE that if that's not the case.

llvm/lib/Transforms/Utils/ProfileVerify.cpp

llvm/test/Transforms/PGOProfile/prof-verify-existing.ll

jdenny-ornl · 2025-07-17T00:16:17Z

llvm/test/Transforms/PGOProfile/prof-verify-as-needed.ll

+; CHECK: br i1 %c, label %yes, label %no, !prof !0
+; CHECK: br i1 %c, label %yes2, label %no, !prof !1
+; CHECK: !0 = !{!"branch_weights", i32 1, i32 2}
+; CHECK: !1 = !{!"branch_weights", i32 429496729, i32 715827882}


Can you conceptually explain to me where these weights come from, or give me a link? I didn't follow the calculation in inject.

In inject, we get the Branch Probability Info. If there's no profile info, that analysis will compute some guesstimate (we can use profi later, but same idea). Inject will turn around and insert those branch probabilities as metadata.

I was looking for an explanation of the guesstimate. Because I don't understand inject's calculation, these new branch weights look arbitrary to me.

Oh! I'd look at BranchProbabilityInfo::estimateBlockWeights for the guesstimate, see also https://reviews.llvm.org/D79485. I'm not deeply familiar with the heuristic, and the goal of this patch isn't dependent on it - we just care about presence/absence. Later, we can make the heuristic pluggable (I was mentioning profi earlier), if we want to do any quantitative analysis.

So right now these are those numbers passes would see when looking at BPI, serialized in metadata.

llvm/include/llvm/Transforms/Utils/ProfileVerify.h

snehasish · 2025-07-21T23:12:25Z

llvm/lib/Transforms/Utils/ProfileVerify.cpp

+public:
+  static bool supportsBranchWeights(const Instruction &I) {
+    return isa<BranchInst>(&I) ||
+


Should this be BranchInst and Conditional? Also drop the extra new lines?

Technically, but this is used internally here after we check that a BB has more than 1 successor. I did something else, ptal.

I like the new approach. Added a suggestion for the check inside it.

llvm/lib/Transforms/Utils/ProfileVerify.cpp

snehasish · 2025-07-22T17:17:20Z

llvm/lib/Transforms/Utils/ProfileVerify.cpp

+public:
+  static bool supportsBranchWeights(const Instruction &I) {
+    return isa<BranchInst>(&I) ||
+


I like the new approach. Added a suggestion for the check inside it.

llvm/lib/Transforms/Utils/ProfileVerify.cpp

llvm/test/Transforms/PGOProfile/prof-verify-existing.ll

snehasish

lgtm

snehasish · 2025-07-23T16:55:04Z

llvm/lib/Transforms/Utils/ProfileVerify.cpp

+               : nullptr;
+  }
+
+  static Instruction *getTerminatorBenefitingFromMDProf(BasicBlock &BB) {


Can the callsite which needs a non-const version const_cast it where necessary?

That would delegate the knowledge about the implementation of this API - the fact that itself doesn't mutate - to the caller.

llvm/lib/Transforms/Utils/ProfileVerify.cpp

github-actions · 2025-07-23T17:05:45Z

✅ With the latest revision this PR passed the C/C++ code formatter.

mtrofin · 2025-07-23T19:33:27Z

Merge activity

Jul 23, 7:33 PM UTC: A user started a stack merge that includes this pull request via Graphite.
Jul 23, 7:35 PM UTC: @mtrofin merged this pull request with Graphite.

llvm-ci · 2025-07-23T21:57:36Z

LLVM Buildbot has detected a new failure on builder lldb-aarch64-windows running on linaro-armv8-windows-msvc-05 while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/10377

Here is the relevant piece of the build log for the reference

Step 6 (test) failure: build (failure)
...
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/2/12 (2272 of 2281)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/3/12 (2273 of 2281)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/4/12 (2274 of 2281)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/5/12 (2275 of 2281)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/6/12 (2276 of 2281)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/7/12 (2277 of 2281)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/8/12 (2278 of 2281)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/9/12 (2279 of 2281)
PASS: lldb-unit :: tools/lldb-server/tests/./LLDBServerTests.exe/0/1 (2280 of 2281)
TIMEOUT: lldb-unit :: Host/./HostTests.exe/6/12 (2281 of 2281)
******************** TEST 'lldb-unit :: Host/./HostTests.exe/6/12' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\unittests\Host\.\HostTests.exe-lldb-unit-13808-6-12.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=12 GTEST_SHARD_INDEX=6 C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\unittests\Host\.\HostTests.exe
--

Note: This is test shard 7 of 12.

[==========] Running 8 tests from 6 test suites.

[----------] Global test environment set-up.

[----------] 1 test from FileSystemTest

[ RUN      ] FileSystemTest.FileAndDirectoryComponents

[       OK ] FileSystemTest.FileAndDirectoryComponents (0 ms)

[----------] 1 test from FileSystemTest (0 ms total)



[----------] 1 test from HostInfoTest

[ RUN      ] HostInfoTest.GetAugmentedArchSpec

[       OK ] HostInfoTest.GetAugmentedArchSpec (0 ms)

[----------] 1 test from HostInfoTest (1 ms total)



[----------] 2 tests from MainLoopTest

[ RUN      ] MainLoopTest.ReadPipeObject


--
exit: 15

Adding 2 passes, one to inject `MD_prof` and one to check its presence. A subsequent patch will add these (similar to debugify) to `opt` (and, eventually, a variant of this, to `llc`) Tracking issue: llvm#147390

mtrofin changed the title ~~pass validator~~ [PGO] Add ProfileInjector and ProfileVerifier passes Jul 7, 2025

mtrofin marked this pull request as ready for review July 7, 2025 20:30

llvmbot added PGO Profile Guided Optimizations llvm:transforms labels Jul 7, 2025

mtrofin requested review from david-xl, jdenny-ornl, jdoerfert, MatzeB, snehasish, teresajohnson and xur-llvm July 7, 2025 21:59

david-xl reviewed Jul 7, 2025

View reviewed changes

mtrofin mentioned this pull request Jul 7, 2025

[PGO] Drive profile validator from opt #147418

Merged

mtrofin force-pushed the users/mtrofin/06-30-_pgo_profile_validation branch from 9c22f8c to 99e4436 Compare July 7, 2025 23:17

jdenny-ornl reviewed Jul 17, 2025

View reviewed changes

mtrofin force-pushed the users/mtrofin/06-30-_pgo_profile_validation branch from 99e4436 to 9200ff5 Compare July 21, 2025 15:12

snehasish reviewed Jul 21, 2025

View reviewed changes

mtrofin force-pushed the users/mtrofin/06-30-_pgo_profile_validation branch from 9200ff5 to 5d0b996 Compare July 22, 2025 00:54

snehasish reviewed Jul 22, 2025

View reviewed changes

mtrofin force-pushed the users/mtrofin/06-30-_pgo_profile_validation branch from 5d0b996 to 3df59d9 Compare July 22, 2025 20:58

snehasish approved these changes Jul 23, 2025

View reviewed changes

mtrofin force-pushed the users/mtrofin/06-30-_pgo_profile_validation branch from 3df59d9 to 7594643 Compare July 23, 2025 17:03

pass validator

f4d5327

mtrofin force-pushed the users/mtrofin/06-30-_pgo_profile_validation branch from 7594643 to f4d5327 Compare July 23, 2025 17:47

mtrofin merged commit df2d2d1 into main Jul 23, 2025
9 checks passed

mtrofin deleted the users/mtrofin/06-30-_pgo_profile_validation branch July 23, 2025 19:35

		@@ -0,0 +1,21 @@
		; Test that prof-verify does not modify existing metadata (incl. "unknown")

[PGO] Add ProfileInjector and ProfileVerifier passes #147388

[PGO] Add ProfileInjector and ProfileVerifier passes #147388

Uh oh!

Conversation

mtrofin commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrofin commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 7, 2025

Uh oh!

llvmbot commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrofin Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snehasish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrofin commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

mtrofin commented Jul 7, 2025 •

edited

Loading

mtrofin commented Jul 7, 2025 •

edited

Loading

mtrofin Jul 22, 2025 •

edited

Loading

github-actions bot commented Jul 23, 2025 •

edited

Loading

mtrofin commented Jul 23, 2025 •

edited

Loading