This is an archive of the discontinued LLVM Phabricator instance.

Differential D124880

[BOLT] Fix ICPJumpTablesTopN option use
ClosedPublic

Authored by Amir on May 3 2022, 2:26 PM.

Details

Reviewers
rafauler
maksfb
Commits
rG60957a5a08a5: [BOLT] Fix ICPJumpTablesTopN option use
Summary

Fix non-sensical opts::ICPJumpTablesTopN != 0 ? opts::ICPTopN : opts::ICPTopN.
Refactor/simplify another similar assignment.

Diff Detail

Event Timeline

Amir created this revision.May 3 2022, 2:26 PM
Herald added a reviewer: rafauler. · View Herald TranscriptMay 3 2022, 2:26 PM
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: ayermolo. · View Herald Transcript
Amir requested review of this revision.May 3 2022, 2:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 3 2022, 2:26 PM
Herald added subscribers: llvm-commits, yota9. · View Herald Transcript
Harbormaster completed remote builds in B162558: Diff 426835.May 3 2022, 2:27 PM
Amir added a parent revision: D124879: [BOLT][NFC] Make ICP options naming uniform.May 3 2022, 2:27 PM
rafauler accepted this revision.May 3 2022, 4:57 PM

LGTM

This revision is now accepted and ready to land.May 3 2022, 4:57 PM
Amir added a child revision: D124896: [BOLT][NFC] Make ICP::verifyProfile static.May 3 2022, 7:22 PM
Closed by commit rG60957a5a08a5: [BOLT] Fix ICPJumpTablesTopN option use (authored by Amir). · Explain WhyMay 3 2022, 7:34 PM
This revision was automatically updated to reflect the committed changes.
Amir added a commit: rG60957a5a08a5: [BOLT] Fix ICPJumpTablesTopN option use.
Amir removed a child revision: D124896: [BOLT][NFC] Make ICP::verifyProfile static.Jun 2 2022, 7:08 PM

Revision Contents

PathSize
bolt/
lib/
Passes/
13 lines

Diff 426904

bolt/lib/Passes/IndirectCallPromotion.cpp

Show First 20 Lines • Show All 540 Lines • ▼ Show 20 Lines if (!HotTargets.empty()) {
uint64_t TotalMemAccesses = 0; uint64_t TotalMemAccesses = 0;
for (size_t I = 0; I < HotTargets.size(); ++I) { for (size_t I = 0; I < HotTargets.size(); ++I) {
const uint64_t TargetIndex = findTargetsIndex(HotTargets[I].second); const uint64_t TargetIndex = findTargetsIndex(HotTargets[I].second);
++IndicesPerTarget[Targets[TargetIndex].To.Sym]; ++IndicesPerTarget[Targets[TargetIndex].To.Sym];
TotalMemAccesses += HotTargets[I].first; TotalMemAccesses += HotTargets[I].first;
} }
uint64_t RemainingMemAccesses = TotalMemAccesses; uint64_t RemainingMemAccesses = TotalMemAccesses;
const size_t TopN = const size_t TopN =
opts::ICPJumpTablesTopN != 0 ? opts::ICPTopN : opts::ICPTopN; opts::ICPJumpTablesTopN ? opts::ICPJumpTablesTopN : opts::ICPTopN;
size_t I = 0; size_t I = 0;
for (; I < HotTargets.size(); ++I) { for (; I < HotTargets.size(); ++I) {
const uint64_t MemAccesses = HotTargets[I].first; const uint64_t MemAccesses = HotTargets[I].first;
if (100 * MemAccesses < if (100 * MemAccesses <
TotalMemAccesses * opts::ICPJTTotalPercentThreshold) TotalMemAccesses * opts::ICPJTTotalPercentThreshold)
break; break;
if (100 * MemAccesses < if (100 * MemAccesses <
RemainingMemAccesses * opts::ICPJTRemainingPercentThreshold) RemainingMemAccesses * opts::ICPJTRemainingPercentThreshold)
▲ Show 20 Lines • Show All 379 Lines • ▼ Show 20 Lines if (opts::Verbosity >= 1) {
outs() << "BOLT-INFO: ICP failed in " << *BB.getFunction() << " @ " outs() << "BOLT-INFO: ICP failed in " << *BB.getFunction() << " @ "
<< InstIdx << " in " << BB.getName() << ", calls = " << NumCalls << InstIdx << " in " << BB.getName() << ", calls = " << NumCalls
<< ", targets empty or NumCalls == 0.\n"; << ", targets empty or NumCalls == 0.\n";
} }
return 0; return 0;
} }
size_t TopN = opts::ICPTopN; size_t TopN = opts::ICPTopN;
if (IsJumpTable) { if (IsJumpTable)
if (opts::ICPJumpTablesTopN != 0) TopN = opts::ICPJumpTablesTopN ? opts::ICPJumpTablesTopN : TopN;
TopN = opts::ICPJumpTablesTopN; else
} else if (opts::ICPCallsTopN != 0) { TopN = opts::ICPCallsTopN ? opts::ICPCallsTopN : TopN;
TopN = opts::ICPCallsTopN;
}
const size_t TrialN = TopN ? std::min(TopN, Targets.size()) : Targets.size(); const size_t TrialN = TopN ? std::min(TopN, Targets.size()) : Targets.size();
if (opts::ICPTopCallsites > 0) { if (opts::ICPTopCallsites > 0) {
BinaryContext &BC = BB.getFunction()->getBinaryContext(); BinaryContext &BC = BB.getFunction()->getBinaryContext();
if (!BC.MIB->hasAnnotation(Inst, "DoICP")) if (!BC.MIB->hasAnnotation(Inst, "DoICP"))
return 0; return 0;
} }
▲ Show 20 Lines • Show All 472 Lines • Show Last 20 Lines