Skip to content

Commit 344228e

Browse files
committed
[BOLT] Drop macro-fusion alignment (#97358)
9d0754a dropped MC support required for optimal macro-fusion alignment in BOLT. Remove the support in BOLT as performance measurements with large binaries didn't show a significant improvement. Test Plan: macro-fusion alignment was never upstreamed, so no upstream tests are affected.
1 parent 1f7d31e commit 344228e

11 files changed

+0
-205
lines changed

bolt/include/bolt/Core/BinaryBasicBlock.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -842,15 +842,6 @@ class BinaryBasicBlock {
842842
bool analyzeBranch(const MCSymbol *&TBB, const MCSymbol *&FBB,
843843
MCInst *&CondBranch, MCInst *&UncondBranch);
844844

845-
/// Return true if iterator \p I is pointing to the first instruction in
846-
/// a pair that could be macro-fused.
847-
bool isMacroOpFusionPair(const_iterator I) const;
848-
849-
/// If the basic block has a pair of instructions suitable for macro-fusion,
850-
/// return iterator to the first instruction of the pair.
851-
/// Otherwise return end().
852-
const_iterator getMacroOpFusionPair() const;
853-
854845
/// Printer required for printing dominator trees.
855846
void printAsOperand(raw_ostream &OS, bool PrintType = true) {
856847
if (PrintType)

bolt/include/bolt/Core/BinaryContext.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -698,10 +698,6 @@ class BinaryContext {
698698

699699
/// Binary-wide aggregated stats.
700700
struct BinaryStats {
701-
/// Stats for macro-fusion.
702-
uint64_t MissedMacroFusionPairs{0};
703-
uint64_t MissedMacroFusionExecCount{0};
704-
705701
/// Stats for stale profile matching:
706702
/// the total number of basic blocks in the profile
707703
uint32_t NumStaleBlocks{0};

bolt/include/bolt/Core/BinaryFunction.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -835,10 +835,6 @@ class BinaryFunction {
835835
/// them.
836836
void calculateLoopInfo();
837837

838-
/// Calculate missed macro-fusion opportunities and update BinaryContext
839-
/// stats.
840-
void calculateMacroOpFusionStats();
841-
842838
/// Returns if BinaryDominatorTree has been constructed for this function.
843839
bool hasDomTree() const { return BDT != nullptr; }
844840

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -930,13 +930,6 @@ class MCPlusBuilder {
930930
/// Return true if the instruction is encoded using EVEX (AVX-512).
931931
virtual bool hasEVEXEncoding(const MCInst &Inst) const { return false; }
932932

933-
/// Return true if a pair of instructions represented by \p Insts
934-
/// could be fused into a single uop.
935-
virtual bool isMacroOpFusionPair(ArrayRef<MCInst> Insts) const {
936-
llvm_unreachable("not implemented");
937-
return false;
938-
}
939-
940933
struct X86MemOperand {
941934
unsigned BaseRegNum;
942935
int64_t ScaleImm;

bolt/lib/Core/BinaryBasicBlock.cpp

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -404,45 +404,6 @@ bool BinaryBasicBlock::analyzeBranch(const MCSymbol *&TBB, const MCSymbol *&FBB,
404404
CondBranch, UncondBranch);
405405
}
406406

407-
bool BinaryBasicBlock::isMacroOpFusionPair(const_iterator I) const {
408-
auto &MIB = Function->getBinaryContext().MIB;
409-
ArrayRef<MCInst> Insts = Instructions;
410-
return MIB->isMacroOpFusionPair(Insts.slice(I - begin()));
411-
}
412-
413-
BinaryBasicBlock::const_iterator
414-
BinaryBasicBlock::getMacroOpFusionPair() const {
415-
if (!Function->getBinaryContext().isX86())
416-
return end();
417-
418-
if (getNumNonPseudos() < 2 || succ_size() != 2)
419-
return end();
420-
421-
auto RI = getLastNonPseudo();
422-
assert(RI != rend() && "cannot have an empty block with 2 successors");
423-
424-
BinaryContext &BC = Function->getBinaryContext();
425-
426-
// Skip instruction if it's an unconditional branch following
427-
// a conditional one.
428-
if (BC.MIB->isUnconditionalBranch(*RI))
429-
++RI;
430-
431-
if (!BC.MIB->isConditionalBranch(*RI))
432-
return end();
433-
434-
// Start checking with instruction preceding the conditional branch.
435-
++RI;
436-
if (RI == rend())
437-
return end();
438-
439-
auto II = std::prev(RI.base()); // convert to a forward iterator
440-
if (isMacroOpFusionPair(II))
441-
return II;
442-
443-
return end();
444-
}
445-
446407
MCInst *BinaryBasicBlock::getTerminatorBefore(MCInst *Pos) {
447408
BinaryContext &BC = Function->getBinaryContext();
448409
auto Itr = rbegin();

bolt/lib/Core/BinaryEmitter.cpp

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,6 @@ extern cl::opt<bool> PreserveBlocksAlignment;
3838
cl::opt<bool> AlignBlocks("align-blocks", cl::desc("align basic blocks"),
3939
cl::cat(BoltOptCategory));
4040

41-
cl::opt<MacroFusionType>
42-
AlignMacroOpFusion("align-macro-fusion",
43-
cl::desc("fix instruction alignment for macro-fusion (x86 relocation mode)"),
44-
cl::init(MFT_HOT),
45-
cl::values(clEnumValN(MFT_NONE, "none",
46-
"do not insert alignment no-ops for macro-fusion"),
47-
clEnumValN(MFT_HOT, "hot",
48-
"only insert alignment no-ops on hot execution paths (default)"),
49-
clEnumValN(MFT_ALL, "all",
50-
"always align instructions to allow macro-fusion")),
51-
cl::ZeroOrMore,
52-
cl::cat(BoltRelocCategory));
53-
5441
static cl::list<std::string>
5542
BreakFunctionNames("break-funcs",
5643
cl::CommaSeparated,
@@ -453,20 +440,7 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
453440
Streamer.emitLabel(EntrySymbol);
454441
}
455442

456-
// Check if special alignment for macro-fusion is needed.
457-
bool MayNeedMacroFusionAlignment =
458-
(opts::AlignMacroOpFusion == MFT_ALL) ||
459-
(opts::AlignMacroOpFusion == MFT_HOT && BB->getKnownExecutionCount());
460-
BinaryBasicBlock::const_iterator MacroFusionPair;
461-
if (MayNeedMacroFusionAlignment) {
462-
MacroFusionPair = BB->getMacroOpFusionPair();
463-
if (MacroFusionPair == BB->end())
464-
MayNeedMacroFusionAlignment = false;
465-
}
466-
467443
SMLoc LastLocSeen;
468-
// Remember if the last instruction emitted was a prefix.
469-
bool LastIsPrefix = false;
470444
for (auto I = BB->begin(), E = BB->end(); I != E; ++I) {
471445
MCInst &Instr = *I;
472446

@@ -479,16 +453,6 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
479453
continue;
480454
}
481455

482-
// Handle macro-fusion alignment. If we emitted a prefix as
483-
// the last instruction, we should've already emitted the associated
484-
// alignment hint, so don't emit it twice.
485-
if (MayNeedMacroFusionAlignment && !LastIsPrefix &&
486-
I == MacroFusionPair) {
487-
// This assumes the second instruction in the macro-op pair will get
488-
// assigned to its own MCRelaxableFragment. Since all JCC instructions
489-
// are relaxable, we should be safe.
490-
}
491-
492456
if (!EmitCodeOnly) {
493457
// A symbol to be emitted before the instruction to mark its location.
494458
MCSymbol *InstrLabel = BC.MIB->getInstLabel(Instr);
@@ -525,7 +489,6 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
525489
}
526490

527491
Streamer.emitInstruction(Instr, *BC.STI);
528-
LastIsPrefix = BC.MIB->isPrefix(Instr);
529492
}
530493
}
531494

bolt/lib/Core/BinaryFunction.cpp

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2279,8 +2279,6 @@ void BinaryFunction::postProcessCFG() {
22792279
postProcessBranches();
22802280
}
22812281

2282-
calculateMacroOpFusionStats();
2283-
22842282
// The final cleanup of intermediate structures.
22852283
clearList(IgnoredBranches);
22862284

@@ -2297,29 +2295,6 @@ void BinaryFunction::postProcessCFG() {
22972295
"invalid CFG detected after post-processing");
22982296
}
22992297

2300-
void BinaryFunction::calculateMacroOpFusionStats() {
2301-
if (!getBinaryContext().isX86())
2302-
return;
2303-
for (const BinaryBasicBlock &BB : blocks()) {
2304-
auto II = BB.getMacroOpFusionPair();
2305-
if (II == BB.end())
2306-
continue;
2307-
2308-
// Check offset of the second instruction.
2309-
// FIXME: arch-specific.
2310-
const uint32_t Offset = BC.MIB->getOffsetWithDefault(*std::next(II), 0);
2311-
if (!Offset || (getAddress() + Offset) % 64)
2312-
continue;
2313-
2314-
LLVM_DEBUG(dbgs() << "\nmissed macro-op fusion at address 0x"
2315-
<< Twine::utohexstr(getAddress() + Offset)
2316-
<< " in function " << *this << "; executed "
2317-
<< BB.getKnownExecutionCount() << " times.\n");
2318-
++BC.Stats.MissedMacroFusionPairs;
2319-
BC.Stats.MissedMacroFusionExecCount += BB.getKnownExecutionCount();
2320-
}
2321-
}
2322-
23232298
void BinaryFunction::removeTagsFromProfile() {
23242299
for (BinaryBasicBlock *BB : BasicBlocks) {
23252300
if (BB->ExecutionCount == BinaryBasicBlock::COUNT_NO_PROFILE)

bolt/lib/Passes/BinaryPasses.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ namespace opts {
4444
extern cl::OptionCategory BoltCategory;
4545
extern cl::OptionCategory BoltOptCategory;
4646

47-
extern cl::opt<bolt::MacroFusionType> AlignMacroOpFusion;
4847
extern cl::opt<unsigned> Verbosity;
4948
extern cl::opt<bool> EnableBAT;
5049
extern cl::opt<unsigned> ExecutionCountThreshold;
@@ -1637,25 +1636,6 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
16371636
}
16381637
}
16391638

1640-
// Print information on missed macro-fusion opportunities seen on input.
1641-
if (BC.Stats.MissedMacroFusionPairs) {
1642-
BC.outs() << format(
1643-
"BOLT-INFO: the input contains %zu (dynamic count : %zu)"
1644-
" opportunities for macro-fusion optimization",
1645-
BC.Stats.MissedMacroFusionPairs, BC.Stats.MissedMacroFusionExecCount);
1646-
switch (opts::AlignMacroOpFusion) {
1647-
case MFT_NONE:
1648-
BC.outs() << ". Use -align-macro-fusion to fix.\n";
1649-
break;
1650-
case MFT_HOT:
1651-
BC.outs() << ". Will fix instances on a hot path.\n";
1652-
break;
1653-
case MFT_ALL:
1654-
BC.outs() << " that are going to be fixed\n";
1655-
break;
1656-
}
1657-
}
1658-
16591639
// Collect and print information about suboptimal code layout on input.
16601640
if (opts::ReportBadLayout) {
16611641
std::vector<BinaryFunction *> SuboptimalFuncs;

bolt/lib/Rewrite/RewriteInstance.cpp

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ extern cl::opt<bool> X86AlignBranchWithin32BBoundaries;
7575

7676
namespace opts {
7777

78-
extern cl::opt<MacroFusionType> AlignMacroOpFusion;
7978
extern cl::list<std::string> HotTextMoveSections;
8079
extern cl::opt<bool> Hugify;
8180
extern cl::opt<bool> Instrument;
@@ -1969,12 +1968,6 @@ void RewriteInstance::adjustCommandLineOptions() {
19691968
if (RuntimeLibrary *RtLibrary = BC->getRuntimeLibrary())
19701969
RtLibrary->adjustCommandLineOptions(*BC);
19711970

1972-
if (opts::AlignMacroOpFusion != MFT_NONE && !BC->isX86()) {
1973-
BC->outs()
1974-
<< "BOLT-INFO: disabling -align-macro-fusion on non-x86 platform\n";
1975-
opts::AlignMacroOpFusion = MFT_NONE;
1976-
}
1977-
19781971
if (BC->isX86() && BC->MAB->allowAutoPadding()) {
19791972
if (!BC->HasRelocations) {
19801973
BC->errs()
@@ -1985,13 +1978,6 @@ void RewriteInstance::adjustCommandLineOptions() {
19851978
BC->outs()
19861979
<< "BOLT-WARNING: using mitigation for Intel JCC erratum, layout "
19871980
"may take several minutes\n";
1988-
opts::AlignMacroOpFusion = MFT_NONE;
1989-
}
1990-
1991-
if (opts::AlignMacroOpFusion != MFT_NONE && !BC->HasRelocations) {
1992-
BC->outs() << "BOLT-INFO: disabling -align-macro-fusion in non-relocation "
1993-
"mode\n";
1994-
opts::AlignMacroOpFusion = MFT_NONE;
19951981
}
19961982

19971983
if (opts::SplitEH && !BC->HasRelocations) {
@@ -2013,14 +1999,6 @@ void RewriteInstance::adjustCommandLineOptions() {
20131999
opts::StrictMode = true;
20142000
}
20152001

2016-
if (BC->isX86() && BC->HasRelocations &&
2017-
opts::AlignMacroOpFusion == MFT_HOT && !ProfileReader) {
2018-
BC->outs()
2019-
<< "BOLT-INFO: enabling -align-macro-fusion=all since no profile "
2020-
"was specified\n";
2021-
opts::AlignMacroOpFusion = MFT_ALL;
2022-
}
2023-
20242002
if (!BC->HasRelocations &&
20252003
opts::ReorderFunctions != ReorderFunctions::RT_NONE) {
20262004
BC->errs() << "BOLT-ERROR: function reordering only works when "

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,6 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
141141
*AArch64ExprB.getSubExpr(), Comp);
142142
}
143143

144-
bool isMacroOpFusionPair(ArrayRef<MCInst> Insts) const override {
145-
return false;
146-
}
147-
148144
bool shortenInstruction(MCInst &, const MCSubtargetInfo &) const override {
149145
return false;
150146
}

bolt/lib/Target/X86/X86MCPlusBuilder.cpp

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -661,40 +661,6 @@ class X86MCPlusBuilder : public MCPlusBuilder {
661661
return (Desc.TSFlags & X86II::EncodingMask) == X86II::EVEX;
662662
}
663663

664-
bool isMacroOpFusionPair(ArrayRef<MCInst> Insts) const override {
665-
const auto *I = Insts.begin();
666-
while (I != Insts.end() && isPrefix(*I))
667-
++I;
668-
if (I == Insts.end())
669-
return false;
670-
671-
const MCInst &FirstInst = *I;
672-
++I;
673-
while (I != Insts.end() && isPrefix(*I))
674-
++I;
675-
if (I == Insts.end())
676-
return false;
677-
const MCInst &SecondInst = *I;
678-
679-
if (!isConditionalBranch(SecondInst))
680-
return false;
681-
// Cannot fuse if the first instruction uses RIP-relative memory.
682-
if (hasPCRelOperand(FirstInst))
683-
return false;
684-
685-
const X86::FirstMacroFusionInstKind CmpKind =
686-
X86::classifyFirstOpcodeInMacroFusion(FirstInst.getOpcode());
687-
if (CmpKind == X86::FirstMacroFusionInstKind::Invalid)
688-
return false;
689-
690-
X86::CondCode CC = static_cast<X86::CondCode>(getCondCode(SecondInst));
691-
X86::SecondMacroFusionInstKind BranchKind =
692-
X86::classifySecondCondCodeInMacroFusion(CC);
693-
if (BranchKind == X86::SecondMacroFusionInstKind::Invalid)
694-
return false;
695-
return X86::isMacroFused(CmpKind, BranchKind);
696-
}
697-
698664
std::optional<X86MemOperand>
699665
evaluateX86MemoryOperand(const MCInst &Inst) const override {
700666
int MemOpNo = getMemoryOperandNo(Inst);

0 commit comments

Comments
 (0)