-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[PGO] Add ProfileInjector and ProfileVerifier passes #147388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
//===- ProfileVerify.h - Verify profile info for testing ----------*-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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
//===- 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/Instructions.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 const Instruction * | ||
getTerminatorBenefitingFromMDProf(const BasicBlock &BB) { | ||
if (succ_size(&BB) < 2) | ||
return nullptr; | ||
auto *Term = BB.getTerminator(); | ||
return (isa<BranchInst>(Term) || isa<SwitchInst>(Term) || | ||
isa<IndirectBrInst>(Term) || isa<CallBrInst>(Term)) | ||
? Term | ||
: nullptr; | ||
} | ||
|
||
static Instruction *getTerminatorBenefitingFromMDProf(BasicBlock &BB) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the callsite which needs a non-const version const_cast it where necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would delegate the knowledge about the implementation of this API - the fact that itself doesn't mutate - to the caller. |
||
return const_cast<Instruction *>( | ||
getTerminatorBenefitingFromMDProf(const_cast<const BasicBlock &>(BB))); | ||
} | ||
|
||
ProfileInjector(Function &F, FunctionAnalysisManager &FAM) : F(F), FAM(FAM) {} | ||
bool inject(); | ||
}; | ||
} // namespace | ||
|
||
// FIXME: currently this injects only for terminators. Select isn't yet | ||
// supported. | ||
bool ProfileInjector::inject() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can SamplePGO's weight propagation be reused here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (capturing offline chat) this would be e.g. profi, and yes, we could use it in a number of ways:
mtrofin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Get whatever branch probability info can be derived from the given IR - | ||
// whether it has or not metadata. The main intention for this pass is to | ||
// ensure that other passes don't drop or "forget" to update MD_prof. We do | ||
// this as a mode in which lit tests would run. We want to avoid changing the | ||
// behavior of those tests. A pass may use BPI (or BFI, which is computed from | ||
// BPI). If no metadata is present, BPI is guesstimated by | ||
// BranchProbabilityAnalysis. The injector (this pass) only persists whatever | ||
// information the analysis provides, in other words, the pass being tested | ||
// will get the same BPI it does if the injector wasn't running. | ||
auto &BPI = FAM.getResult<BranchProbabilityAnalysis>(F); | ||
|
||
bool Changed = false; | ||
for (auto &BB : F) { | ||
auto *Term = getTerminatorBenefitingFromMDProf(BB); | ||
mtrofin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!Term || Term->getMetadata(LLVMContext::MD_prof)) | ||
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))); | ||
|
||
assert(llvm::find_if(Probs, | ||
[](const BranchProbability &P) { | ||
return P.isUnknown(); | ||
}) == Probs.end() && | ||
"All branch probabilities should be valid"); | ||
const auto *FirstZeroDenominator = | ||
find_if(Probs, [](const BranchProbability &P) { | ||
return P.getDenominator() == 0; | ||
}); | ||
Comment on lines
+81
to
+84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard with NDEBUG? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, because only used in assert - done (something similar) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
(void)FirstZeroDenominator; | ||
assert(FirstZeroDenominator == Probs.end()); | ||
const auto *FirstNonZeroNumerator = | ||
find_if(Probs, [](const BranchProbability &P) { return !P.isZero(); }); | ||
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())); | ||
mtrofin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
GCD = llvm::gcd(GCD, DynamicAPInt(Prob.getNumerator())); | ||
} | ||
SmallVector<uint32_t> Weights; | ||
Weights.reserve(Term->getNumSuccessors()); | ||
for (const auto &Prob : Probs) { | ||
DynamicAPInt W = | ||
(Prob.getNumerator() * LCM / GCD) / Prob.getDenominator(); | ||
Weights.emplace_back(static_cast<uint32_t>((int64_t)W)); | ||
} | ||
setBranchWeights(*Term, Weights, /*IsExpected=*/false); | ||
Changed = true; | ||
} | ||
return Changed; | ||
} | ||
|
||
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) { | ||
for (const auto &BB : F) | ||
if (const auto *Term = | ||
ProfileInjector::getTerminatorBenefitingFromMDProf(BB)) | ||
if (!Term->getMetadata(LLVMContext::MD_prof)) | ||
F.getContext().emitError("Profile verification failed"); | ||
|
||
return PreservedAnalyses::none(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
; 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 | ||
mtrofin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
; CHECK: br i1 %c, label %yes2, label %no, !prof !1 | ||
; CHECK: !0 = !{!"branch_weights", i32 1, i32 2} | ||
; CHECK: !1 = !{!"branch_weights", i32 3, i32 5} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
; Test that prof-inject 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 | ||
snehasish marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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"} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 3, i32 5} | ||
|
||
; VERIFY: Profile verification failed |
Uh oh!
There was an error while loading. Please reload this page.