Skip to content

Commit

Permalink
Revert "[BOLT] Hash-based function matching (#95821)"
Browse files Browse the repository at this point in the history
This reverts commit 5e097c7.
  • Loading branch information
shawbyoung authored Jun 24, 2024
1 parent 5e097c7 commit acb7a1b
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 135 deletions.
4 changes: 0 additions & 4 deletions bolt/docs/CommandLineArgumentReference.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,6 @@

Always use long jumps/nops for Linux kernel static keys

- `--match-profile-with-function-hash`

Match profile with function hash

- `--max-data-relocations=<uint>`

Maximum number of data relocations to process
Expand Down
68 changes: 10 additions & 58 deletions bolt/lib/Profile/YAMLProfileReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ namespace opts {
extern cl::opt<unsigned> Verbosity;
extern cl::OptionCategory BoltOptCategory;
extern cl::opt<bool> InferStaleProfile;
extern cl::opt<bool> MatchProfileWithFunctionHash;
extern cl::opt<bool> Lite;

static llvm::cl::opt<bool>
IgnoreHash("profile-ignore-hash",
Expand Down Expand Up @@ -365,19 +363,9 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
return Profile.Hash == static_cast<uint64_t>(BF.getHash());
};

uint64_t MatchedWithExactName = 0;
uint64_t MatchedWithHash = 0;
uint64_t MatchedWithLTOCommonName = 0;

// Computes hash for binary functions.
if (opts::MatchProfileWithFunctionHash)
for (auto &[_, BF] : BC.getBinaryFunctions())
BF.computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);
else if (!opts::IgnoreHash)
for (BinaryFunction *BF : ProfileBFs)
BF->computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);

// This first pass assigns profiles that match 100% by name and by hash.
// We have to do 2 passes since LTO introduces an ambiguity in function
// names. The first pass assigns profiles that match 100% by name and
// by hash. The second pass allows name ambiguity for LTO private functions.
for (auto [YamlBF, BF] : llvm::zip_equal(YamlBP.Functions, ProfileBFs)) {
if (!BF)
continue;
Expand All @@ -386,34 +374,15 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
// the profile.
Function.setExecutionCount(BinaryFunction::COUNT_NO_PROFILE);

if (profileMatches(YamlBF, Function)) {
matchProfileToFunction(YamlBF, Function);
++MatchedWithExactName;
}
}

// Uses the strict hash of profiled and binary functions to match functions
// that are not matched by name or common name.
if (opts::MatchProfileWithFunctionHash) {
std::unordered_map<size_t, BinaryFunction *> StrictHashToBF;
StrictHashToBF.reserve(BC.getBinaryFunctions().size());
// Recompute hash once per function.
if (!opts::IgnoreHash)
Function.computeHash(YamlBP.Header.IsDFSOrder,
YamlBP.Header.HashFunction);

for (auto &[_, BF] : BC.getBinaryFunctions())
StrictHashToBF[BF.getHash()] = &BF;

for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
if (YamlBF.Used)
continue;
auto It = StrictHashToBF.find(YamlBF.Hash);
if (It != StrictHashToBF.end() && !ProfiledFunctions.count(It->second)) {
BinaryFunction *BF = It->second;
matchProfileToFunction(YamlBF, *BF);
++MatchedWithHash;
}
}
if (profileMatches(YamlBF, Function))
matchProfileToFunction(YamlBF, Function);
}

// This second pass allows name ambiguity for LTO private functions.
for (const auto &[CommonName, LTOProfiles] : LTOCommonNameMap) {
if (!LTOCommonNameFunctionMap.contains(CommonName))
continue;
Expand All @@ -427,7 +396,6 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
for (BinaryFunction *BF : Functions) {
if (!ProfiledFunctions.count(BF) && profileMatches(*YamlBF, *BF)) {
matchProfileToFunction(*YamlBF, *BF);
++MatchedWithLTOCommonName;
return true;
}
}
Expand All @@ -439,10 +407,8 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
// partially.
if (!ProfileMatched && LTOProfiles.size() == 1 && Functions.size() == 1 &&
!LTOProfiles.front()->Used &&
!ProfiledFunctions.count(*Functions.begin())) {
!ProfiledFunctions.count(*Functions.begin()))
matchProfileToFunction(*LTOProfiles.front(), **Functions.begin());
++MatchedWithLTOCommonName;
}
}

for (auto [YamlBF, BF] : llvm::zip_equal(YamlBP.Functions, ProfileBFs))
Expand All @@ -454,15 +420,6 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
errs() << "BOLT-WARNING: profile ignored for function " << YamlBF.Name
<< '\n';

if (opts::Verbosity >= 2) {
outs() << "BOLT-INFO: matched " << MatchedWithExactName
<< " functions with identical names\n";
outs() << "BOLT-INFO: matched " << MatchedWithHash
<< " functions with hash\n";
outs() << "BOLT-INFO: matched " << MatchedWithLTOCommonName
<< " functions with matching LTO common names\n";
}

// Set for parseFunctionProfile().
NormalizeByInsnCount = usesEvent("cycles") || usesEvent("instructions");
NormalizeByCalls = usesEvent("branches");
Expand All @@ -482,11 +439,6 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {

BC.setNumUnusedProfiledObjects(NumUnused);

if (opts::Lite)
for (BinaryFunction *BF : BC.getAllBinaryFunctions())
if (!BF->hasProfile())
BF->setIgnored();

return Error::success();
}

Expand Down
4 changes: 0 additions & 4 deletions bolt/lib/Rewrite/RewriteInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ extern cl::opt<bool> Hugify;
extern cl::opt<bool> Instrument;
extern cl::opt<JumpTableSupportLevel> JumpTables;
extern cl::opt<bool> KeepNops;
extern cl::opt<bool> MatchProfileWithFunctionHash;
extern cl::list<std::string> ReorderData;
extern cl::opt<bolt::ReorderFunctions::ReorderType> ReorderFunctions;
extern cl::opt<bool> TerminalTrap;
Expand Down Expand Up @@ -2983,9 +2982,6 @@ void RewriteInstance::selectFunctionsToProcess() {
if (mustSkip(Function))
return false;

if (opts::MatchProfileWithFunctionHash)
return true;

// If the list is not empty, only process functions from the list.
if (!opts::ForceFunctionNames.empty() || !ForceFunctionsNR.empty()) {
// Regex check (-funcs and -funcs-file options).
Expand Down
5 changes: 0 additions & 5 deletions bolt/lib/Utils/CommandLineOpts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,6 @@ cl::opt<bool>
cl::desc("instrument code to generate accurate profile data"),
cl::cat(BoltOptCategory));

cl::opt<bool>
MatchProfileWithFunctionHash("match-profile-with-function-hash",
cl::desc("Match profile with function hash"),
cl::Hidden, cl::cat(BoltCategory));

cl::opt<std::string>
OutputFilename("o",
cl::desc("<output file>"),
Expand Down
64 changes: 0 additions & 64 deletions bolt/test/X86/hashing-based-function-matching.test

This file was deleted.

0 comments on commit acb7a1b

Please sign in to comment.