This is an archive of the discontinued LLVM Phabricator instance.

extending code layout alg
ClosedPublic

Authored by spupyrev on Jul 15 2022, 12:23 PM.

Details

Summary

The diff modifies ext-tsp code layout algorithm in the following ways:
(i) fixes merging of cold block chains (this is a port of D129397);
(ii) adjusts the cost model utilized for optimization;
(iii) adjusts some APIs so that the implementation can be used in BOLT; this is
a prerequisite for D129895.

The only non-trivial change is (ii). Here we introduce different weights for
conditional and unconditional branches in the cost model. Based on the new model
it is slightly more important to increase the number of "fall-through
unconditional" jumps, which makes sense, as placing two blocks with an
unconditional jump next to each other reduces the number of jump instructions in
the generated code. Experimentally, this makes a mild impact on the performance;
I've seen up to 0.2%-0.3% perf win on some benchmarks.

Diff Detail

Event Timeline

spupyrev created this revision.Jul 15 2022, 12:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2022, 12:23 PM
spupyrev requested review of this revision.Jul 15 2022, 12:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2022, 12:23 PM
spupyrev edited the summary of this revision. (Show Details)Jul 15 2022, 12:40 PM
spupyrev added reviewers: hoy, wenlei, maksfb, rafauler, wlei.
Matt added a subscriber: Matt.Aug 9 2022, 11:00 PM
hoy added inline comments.Aug 19 2022, 5:00 PM
llvm/lib/CodeGen/MachineBlockPlacement.cpp
3491

nit: use EdgeT here instead of std::pair<uint64_t, uint64_t>?

Maybe also define a type, e.g, EdgeCountT for std::pair<std::pair<uint64_t, uint64_t>, uint64_t> ?

llvm/lib/Transforms/Utils/CodeLayout.cpp
133

nit: return jumpExtTSPScore(0, 1, Count, IsConditional ? FallthroughWeightCond : FallthroughWeightUncond);

407–410

Why do you need 'const` here?

hoy accepted this revision.Aug 23 2022, 12:02 PM

LGTM, thanks.

This revision is now accepted and ready to land.Aug 23 2022, 12:02 PM

lgtm with a few nits.

llvm/lib/Transforms/Utils/CodeLayout.cpp
513–514

nit: SuccNodes.resize(NumNodes) should just work, so you don't invoke copy/move constructor here. Same for PredNodes.

515

nit: std::vector<uint64_t> OutDegree(NumNodes, 0); this is avoids copy/move constructor from the temp object.

Same for the other auto OutDegree and other instances of auto xx = std::vector<..>.

spupyrev updated this revision to Diff 455218.Aug 24 2022, 7:57 AM

more suggested changes + rebase

This revision was landed with ongoing or failed builds.Aug 24 2022, 9:41 AM
This revision was automatically updated to reflect the committed changes.