Skip to content

Commit 2477cec

Browse files
KanRobertKanRobert
committed
[NFC][X86] Refine code in X86AsmBackend
Summary: Move code to a better place, rename function, etc Tags: #llvm Differential Revision: https://reviews.llvm.org/D77778
1 parent 9aa5fbb commit 2477cec

File tree

1 file changed

+75
-84
lines changed

1 file changed

+75
-84
lines changed

‎llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp‎

Lines changed: 75 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,9 @@ class X86AlignBranchKind {
6262
else if (BranchType == "indirect")
6363
addKind(X86::AlignBranchIndirect);
6464
else {
65-
report_fatal_error(
66-
"'-x86-align-branch 'The branches's type is combination of jcc, "
67-
"fused, jmp, call, ret, indirect.(plus separated)",
68-
false);
65+
errs() << "invalid argument " << BranchType.str()
66+
<< " to -x86-align-branch=; each element must be one of: fused, "
67+
"jcc, jmp, call, ret, indirect.(plus separated)";
6968
}
7069
}
7170
}
@@ -129,18 +128,18 @@ class X86AsmBackend : public MCAsmBackend {
129128
std::unique_ptr<const MCInstrInfo> MCII;
130129
X86AlignBranchKind AlignBranchType;
131130
Align AlignBoundary;
131+
unsigned TargetPrefixMax = 0;
132132

133-
uint8_t determinePaddingPrefix(const MCInst &Inst) const;
134-
135-
bool isMacroFused(const MCInst &Cmp, const MCInst &Jcc) const;
136-
137-
bool needAlign(MCObjectStreamer &OS) const;
138-
bool needAlignInst(const MCInst &Inst) const;
139-
bool allowAutoPaddingForInst(const MCInst &Inst, MCObjectStreamer &OS) const;
140133
MCInst PrevInst;
141-
MCBoundaryAlignFragment *PendingBoundaryAlign = nullptr;
134+
MCBoundaryAlignFragment *PendingBA = nullptr;
142135
std::pair<MCFragment *, size_t> PrevInstPosition;
143-
bool AllowAutoPaddingForInst;
136+
bool CanPadInst;
137+
138+
uint8_t determinePaddingPrefix(const MCInst &Inst) const;
139+
bool isMacroFused(const MCInst &Cmp, const MCInst &Jcc) const;
140+
bool needAlign(const MCInst &Inst) const;
141+
bool canPadBranches(MCObjectStreamer &OS) const;
142+
bool canPadInst(const MCInst &Inst, MCObjectStreamer &OS) const;
144143

145144
public:
146145
X86AsmBackend(const Target &T, const MCSubtargetInfo &STI)
@@ -161,6 +160,8 @@ class X86AsmBackend : public MCAsmBackend {
161160
AlignBoundary = assumeAligned(X86AlignBranchBoundary);
162161
if (X86AlignBranch.getNumOccurrences())
163162
AlignBranchType = X86AlignBranchKindLoc;
163+
if (X86PadMaxPrefixSize.getNumOccurrences())
164+
TargetPrefixMax = X86PadMaxPrefixSize;
164165
}
165166

166167
bool allowAutoPadding() const override;
@@ -459,23 +460,7 @@ bool X86AsmBackend::allowAutoPadding() const {
459460
}
460461

461462
bool X86AsmBackend::allowEnhancedRelaxation() const {
462-
return allowAutoPadding() && X86PadMaxPrefixSize != 0 && X86PadForBranchAlign;
463-
}
464-
465-
bool X86AsmBackend::needAlign(MCObjectStreamer &OS) const {
466-
if (!OS.getAllowAutoPadding())
467-
return false;
468-
assert(allowAutoPadding() && "incorrect initialization!");
469-
470-
// To be Done: Currently don't deal with Bundle cases.
471-
if (OS.getAssembler().isBundlingEnabled())
472-
return false;
473-
474-
// Branches only need to be aligned in 32-bit or 64-bit mode.
475-
if (!(STI.hasFeature(X86::Mode64Bit) || STI.hasFeature(X86::Mode32Bit)))
476-
return false;
477-
478-
return true;
463+
return allowAutoPadding() && TargetPrefixMax != 0 && X86PadForBranchAlign;
479464
}
480465

481466
/// X86 has certain instructions which enable interrupts exactly one
@@ -545,25 +530,9 @@ static size_t getSizeForInstFragment(const MCFragment *F) {
545530
}
546531
}
547532

548-
/// Check if the instruction operand needs to be aligned.
549-
bool X86AsmBackend::needAlignInst(const MCInst &Inst) const {
550-
const MCInstrDesc &InstDesc = MCII->get(Inst.getOpcode());
551-
return (InstDesc.isConditionalBranch() &&
552-
(AlignBranchType & X86::AlignBranchJcc)) ||
553-
(InstDesc.isUnconditionalBranch() &&
554-
(AlignBranchType & X86::AlignBranchJmp)) ||
555-
(InstDesc.isCall() &&
556-
(AlignBranchType & X86::AlignBranchCall)) ||
557-
(InstDesc.isReturn() &&
558-
(AlignBranchType & X86::AlignBranchRet)) ||
559-
(InstDesc.isIndirectBranch() &&
560-
(AlignBranchType & X86::AlignBranchIndirect));
561-
}
562-
563533
/// Return true if we can insert NOP or prefixes automatically before the
564534
/// the instruction to be emitted.
565-
bool X86AsmBackend::allowAutoPaddingForInst(const MCInst &Inst,
566-
MCObjectStreamer &OS) const {
535+
bool X86AsmBackend::canPadInst(const MCInst &Inst, MCObjectStreamer &OS) const {
567536
if (hasVariantSymbol(Inst))
568537
// Linker may rewrite the instruction with variant symbol operand(e.g.
569538
// TLSCALL).
@@ -592,23 +561,51 @@ bool X86AsmBackend::allowAutoPaddingForInst(const MCInst &Inst,
592561
return true;
593562
}
594563

564+
bool X86AsmBackend::canPadBranches(MCObjectStreamer &OS) const {
565+
if (!OS.getAllowAutoPadding())
566+
return false;
567+
assert(allowAutoPadding() && "incorrect initialization!");
568+
569+
// To be Done: Currently don't deal with Bundle cases.
570+
if (OS.getAssembler().isBundlingEnabled())
571+
return false;
572+
573+
// Branches only need to be aligned in 32-bit or 64-bit mode.
574+
if (!(STI.hasFeature(X86::Mode64Bit) || STI.hasFeature(X86::Mode32Bit)))
575+
return false;
576+
577+
return true;
578+
}
579+
580+
/// Check if the instruction operand needs to be aligned.
581+
bool X86AsmBackend::needAlign(const MCInst &Inst) const {
582+
const MCInstrDesc &Desc = MCII->get(Inst.getOpcode());
583+
return (Desc.isConditionalBranch() &&
584+
(AlignBranchType & X86::AlignBranchJcc)) ||
585+
(Desc.isUnconditionalBranch() &&
586+
(AlignBranchType & X86::AlignBranchJmp)) ||
587+
(Desc.isCall() && (AlignBranchType & X86::AlignBranchCall)) ||
588+
(Desc.isReturn() && (AlignBranchType & X86::AlignBranchRet)) ||
589+
(Desc.isIndirectBranch() &&
590+
(AlignBranchType & X86::AlignBranchIndirect));
591+
}
592+
595593
/// Insert BoundaryAlignFragment before instructions to align branches.
596594
void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
597595
const MCInst &Inst) {
598-
AllowAutoPaddingForInst = allowAutoPaddingForInst(Inst, OS);
596+
CanPadInst = canPadInst(Inst, OS);
599597

600-
if (!needAlign(OS))
598+
if (!canPadBranches(OS))
601599
return;
602600

603601
if (!isMacroFused(PrevInst, Inst))
604602
// Macro fusion doesn't happen indeed, clear the pending.
605-
PendingBoundaryAlign = nullptr;
603+
PendingBA = nullptr;
606604

607-
if (!AllowAutoPaddingForInst)
605+
if (!CanPadInst)
608606
return;
609607

610-
if (PendingBoundaryAlign &&
611-
OS.getCurrentFragment()->getPrevNode() == PendingBoundaryAlign) {
608+
if (PendingBA && OS.getCurrentFragment()->getPrevNode() == PendingBA) {
612609
// Macro fusion actually happens and there is no other fragment inserted
613610
// after the previous instruction.
614611
//
@@ -630,12 +627,11 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
630627
return;
631628
}
632629

633-
if (needAlignInst(Inst) || ((AlignBranchType & X86::AlignBranchFused) &&
634-
isFirstMacroFusibleInst(Inst, *MCII))) {
630+
if (needAlign(Inst) || ((AlignBranchType & X86::AlignBranchFused) &&
631+
isFirstMacroFusibleInst(Inst, *MCII))) {
635632
// If we meet a unfused branch or the first instuction in a fusiable pair,
636633
// insert a BoundaryAlign fragment.
637-
OS.insert(PendingBoundaryAlign =
638-
new MCBoundaryAlignFragment(AlignBoundary));
634+
OS.insert(PendingBA = new MCBoundaryAlignFragment(AlignBoundary));
639635
}
640636
}
641637

@@ -645,17 +641,17 @@ void X86AsmBackend::emitInstructionEnd(MCObjectStreamer &OS, const MCInst &Inst)
645641
MCFragment *CF = OS.getCurrentFragment();
646642
PrevInstPosition = std::make_pair(CF, getSizeForInstFragment(CF));
647643
if (auto *F = dyn_cast_or_null<MCRelaxableFragment>(CF))
648-
F->setAllowAutoPadding(AllowAutoPaddingForInst);
644+
F->setAllowAutoPadding(CanPadInst);
649645

650-
if (!needAlign(OS))
646+
if (!canPadBranches(OS))
651647
return;
652648

653-
if (!needAlignInst(Inst) || !PendingBoundaryAlign)
649+
if (!needAlign(Inst) || !PendingBA)
654650
return;
655651

656652
// Tie the aligned instructions into a a pending BoundaryAlign.
657-
PendingBoundaryAlign->setLastFragment(CF);
658-
PendingBoundaryAlign = nullptr;
653+
PendingBA->setLastFragment(CF);
654+
PendingBA = nullptr;
659655

660656
// We need to ensure that further data isn't added to the current
661657
// DataFragment, so that we can get the size of instructions later in
@@ -853,26 +849,6 @@ static bool isFullyRelaxed(const MCRelaxableFragment &RF) {
853849
return getRelaxedOpcode(Inst, Is16BitMode) == Inst.getOpcode();
854850
}
855851

856-
static unsigned getRemainingPrefixSize(const MCInst &Inst,
857-
const MCSubtargetInfo &STI,
858-
MCCodeEmitter &Emitter) {
859-
SmallString<256> Code;
860-
raw_svector_ostream VecOS(Code);
861-
Emitter.emitPrefix(Inst, VecOS, STI);
862-
assert(Code.size() < 15 && "The number of prefixes must be less than 15.");
863-
864-
// TODO: It turns out we need a decent amount of plumbing for the target
865-
// specific bits to determine number of prefixes its safe to add. Various
866-
// targets (older chips mostly, but also Atom family) encounter decoder
867-
// stalls with too many prefixes. For testing purposes, we set the value
868-
// externally for the moment.
869-
unsigned ExistingPrefixSize = Code.size();
870-
unsigned TargetPrefixMax = X86PadMaxPrefixSize;
871-
if (TargetPrefixMax <= ExistingPrefixSize)
872-
return 0;
873-
return TargetPrefixMax - ExistingPrefixSize;
874-
}
875-
876852
bool X86AsmBackend::padInstructionViaPrefix(MCRelaxableFragment &RF,
877853
MCCodeEmitter &Emitter,
878854
unsigned &RemainingSize) const {
@@ -890,9 +866,24 @@ bool X86AsmBackend::padInstructionViaPrefix(MCRelaxableFragment &RF,
890866
return false;
891867

892868
const unsigned MaxPossiblePad = std::min(15 - OldSize, RemainingSize);
869+
const unsigned RemainingPrefixSize = [&]() -> unsigned {
870+
SmallString<15> Code;
871+
raw_svector_ostream VecOS(Code);
872+
Emitter.emitPrefix(RF.getInst(), VecOS, STI);
873+
assert(Code.size() < 15 && "The number of prefixes must be less than 15.");
874+
875+
// TODO: It turns out we need a decent amount of plumbing for the target
876+
// specific bits to determine number of prefixes its safe to add. Various
877+
// targets (older chips mostly, but also Atom family) encounter decoder
878+
// stalls with too many prefixes. For testing purposes, we set the value
879+
// externally for the moment.
880+
unsigned ExistingPrefixSize = Code.size();
881+
if (TargetPrefixMax <= ExistingPrefixSize)
882+
return 0;
883+
return TargetPrefixMax - ExistingPrefixSize;
884+
}();
893885
const unsigned PrefixBytesToAdd =
894-
std::min(MaxPossiblePad,
895-
getRemainingPrefixSize(RF.getInst(), STI, Emitter));
886+
std::min(MaxPossiblePad, RemainingPrefixSize);
896887
if (PrefixBytesToAdd == 0)
897888
return false;
898889

0 commit comments

Comments
 (0)