Skip to content

Commit 32e4906

Browse files
authored
Revert "[BOLT] Hash-based function matching" (llvm#96568)
Reverts llvm#95821
1 parent b3c668b commit 32e4906

File tree

5 files changed

+10
-135
lines changed

5 files changed

+10
-135
lines changed

bolt/docs/CommandLineArgumentReference.md

-4
Original file line numberDiff line numberDiff line change
@@ -259,10 +259,6 @@
259259

260260
Always use long jumps/nops for Linux kernel static keys
261261

262-
- `--match-profile-with-function-hash`
263-
264-
Match profile with function hash
265-
266262
- `--max-data-relocations=<uint>`
267263

268264
Maximum number of data relocations to process

bolt/lib/Profile/YAMLProfileReader.cpp

+10-58
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ namespace opts {
2222
extern cl::opt<unsigned> Verbosity;
2323
extern cl::OptionCategory BoltOptCategory;
2424
extern cl::opt<bool> InferStaleProfile;
25-
extern cl::opt<bool> MatchProfileWithFunctionHash;
26-
extern cl::opt<bool> Lite;
2725

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

368-
uint64_t MatchedWithExactName = 0;
369-
uint64_t MatchedWithHash = 0;
370-
uint64_t MatchedWithLTOCommonName = 0;
371-
372-
// Computes hash for binary functions.
373-
if (opts::MatchProfileWithFunctionHash)
374-
for (auto &[_, BF] : BC.getBinaryFunctions())
375-
BF.computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);
376-
else if (!opts::IgnoreHash)
377-
for (BinaryFunction *BF : ProfileBFs)
378-
BF->computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);
379-
380-
// This first pass assigns profiles that match 100% by name and by hash.
366+
// We have to do 2 passes since LTO introduces an ambiguity in function
367+
// names. The first pass assigns profiles that match 100% by name and
368+
// by hash. The second pass allows name ambiguity for LTO private functions.
381369
for (auto [YamlBF, BF] : llvm::zip_equal(YamlBP.Functions, ProfileBFs)) {
382370
if (!BF)
383371
continue;
@@ -386,34 +374,15 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
386374
// the profile.
387375
Function.setExecutionCount(BinaryFunction::COUNT_NO_PROFILE);
388376

389-
if (profileMatches(YamlBF, Function)) {
390-
matchProfileToFunction(YamlBF, Function);
391-
++MatchedWithExactName;
392-
}
393-
}
394-
395-
// Uses the strict hash of profiled and binary functions to match functions
396-
// that are not matched by name or common name.
397-
if (opts::MatchProfileWithFunctionHash) {
398-
std::unordered_map<size_t, BinaryFunction *> StrictHashToBF;
399-
StrictHashToBF.reserve(BC.getBinaryFunctions().size());
377+
// Recompute hash once per function.
378+
if (!opts::IgnoreHash)
379+
Function.computeHash(YamlBP.Header.IsDFSOrder,
380+
YamlBP.Header.HashFunction);
400381

401-
for (auto &[_, BF] : BC.getBinaryFunctions())
402-
StrictHashToBF[BF.getHash()] = &BF;
403-
404-
for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
405-
if (YamlBF.Used)
406-
continue;
407-
auto It = StrictHashToBF.find(YamlBF.Hash);
408-
if (It != StrictHashToBF.end() && !ProfiledFunctions.count(It->second)) {
409-
BinaryFunction *BF = It->second;
410-
matchProfileToFunction(YamlBF, *BF);
411-
++MatchedWithHash;
412-
}
413-
}
382+
if (profileMatches(YamlBF, Function))
383+
matchProfileToFunction(YamlBF, Function);
414384
}
415385

416-
// This second pass allows name ambiguity for LTO private functions.
417386
for (const auto &[CommonName, LTOProfiles] : LTOCommonNameMap) {
418387
if (!LTOCommonNameFunctionMap.contains(CommonName))
419388
continue;
@@ -427,7 +396,6 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
427396
for (BinaryFunction *BF : Functions) {
428397
if (!ProfiledFunctions.count(BF) && profileMatches(*YamlBF, *BF)) {
429398
matchProfileToFunction(*YamlBF, *BF);
430-
++MatchedWithLTOCommonName;
431399
return true;
432400
}
433401
}
@@ -439,10 +407,8 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
439407
// partially.
440408
if (!ProfileMatched && LTOProfiles.size() == 1 && Functions.size() == 1 &&
441409
!LTOProfiles.front()->Used &&
442-
!ProfiledFunctions.count(*Functions.begin())) {
410+
!ProfiledFunctions.count(*Functions.begin()))
443411
matchProfileToFunction(*LTOProfiles.front(), **Functions.begin());
444-
++MatchedWithLTOCommonName;
445-
}
446412
}
447413

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

457-
if (opts::Verbosity >= 2) {
458-
outs() << "BOLT-INFO: matched " << MatchedWithExactName
459-
<< " functions with identical names\n";
460-
outs() << "BOLT-INFO: matched " << MatchedWithHash
461-
<< " functions with hash\n";
462-
outs() << "BOLT-INFO: matched " << MatchedWithLTOCommonName
463-
<< " functions with matching LTO common names\n";
464-
}
465-
466423
// Set for parseFunctionProfile().
467424
NormalizeByInsnCount = usesEvent("cycles") || usesEvent("instructions");
468425
NormalizeByCalls = usesEvent("branches");
@@ -482,11 +439,6 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
482439

483440
BC.setNumUnusedProfiledObjects(NumUnused);
484441

485-
if (opts::Lite)
486-
for (BinaryFunction *BF : BC.getAllBinaryFunctions())
487-
if (!BF->hasProfile())
488-
BF->setIgnored();
489-
490442
return Error::success();
491443
}
492444

bolt/lib/Rewrite/RewriteInstance.cpp

-4
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ extern cl::opt<bool> Hugify;
8282
extern cl::opt<bool> Instrument;
8383
extern cl::opt<JumpTableSupportLevel> JumpTables;
8484
extern cl::opt<bool> KeepNops;
85-
extern cl::opt<bool> MatchProfileWithFunctionHash;
8685
extern cl::list<std::string> ReorderData;
8786
extern cl::opt<bolt::ReorderFunctions::ReorderType> ReorderFunctions;
8887
extern cl::opt<bool> TerminalTrap;
@@ -2983,9 +2982,6 @@ void RewriteInstance::selectFunctionsToProcess() {
29832982
if (mustSkip(Function))
29842983
return false;
29852984

2986-
if (opts::MatchProfileWithFunctionHash)
2987-
return true;
2988-
29892985
// If the list is not empty, only process functions from the list.
29902986
if (!opts::ForceFunctionNames.empty() || !ForceFunctionsNR.empty()) {
29912987
// Regex check (-funcs and -funcs-file options).

bolt/lib/Utils/CommandLineOpts.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,6 @@ cl::opt<bool>
128128
cl::desc("instrument code to generate accurate profile data"),
129129
cl::cat(BoltOptCategory));
130130

131-
cl::opt<bool>
132-
MatchProfileWithFunctionHash("match-profile-with-function-hash",
133-
cl::desc("Match profile with function hash"),
134-
cl::Hidden, cl::cat(BoltCategory));
135-
136131
cl::opt<std::string>
137132
OutputFilename("o",
138133
cl::desc("<output file>"),

bolt/test/X86/hashing-based-function-matching.test

-64
This file was deleted.

0 commit comments

Comments
 (0)