Page MenuHomePhabricator
Differential D70157 Diff 234939 llvm/lib/MC/MCAssembler.cpp

Changeset View

Standalone View

View Options

llvm/lib/MC/MCAssembler.cpp

Show First 20 Lines • Show All 303 Lines • ▼ Show 20 Lines if (Size < 0) {
return 0; return 0;
} }
return Size; return Size;
} }
case MCFragment::FT_LEB: case MCFragment::FT_LEB:
return cast<MCLEBFragment>(F).getContents().size(); return cast<MCLEBFragment>(F).getContents().size();
case MCFragment::FT_BoundaryAlign:
return cast<MCBoundaryAlignFragment>(F).getSize();
case MCFragment::FT_SymbolId: case MCFragment::FT_SymbolId:
return 4; return 4;
case MCFragment::FT_Align: { case MCFragment::FT_Align: {
const MCAlignFragment &AF = cast<MCAlignFragment>(F); const MCAlignFragment &AF = cast<MCAlignFragment>(F);
unsigned Offset = Layout.getFragmentOffset(&AF); unsigned Offset = Layout.getFragmentOffset(&AF);
unsigned Size = offsetToAlignment(Offset, Align(AF.getAlignment())); unsigned Size = offsetToAlignment(Offset, Align(AF.getAlignment()));
▲ Show 20 Lines • Show All 280 Lines • ▼ Show 20 Linesstatic void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
} }
case MCFragment::FT_LEB: { case MCFragment::FT_LEB: {
const MCLEBFragment &LF = cast<MCLEBFragment>(F); const MCLEBFragment &LF = cast<MCLEBFragment>(F);
OS << LF.getContents(); OS << LF.getContents();
break; break;
} }
case MCFragment::FT_BoundaryAlign: {
if (!Asm.getBackend().writeNopData(OS, FragmentSize))
report_fatal_error("unable to write nop sequence of " +
Twine(FragmentSize) + " bytes");
break;
}
case MCFragment::FT_SymbolId: { case MCFragment::FT_SymbolId: {
const MCSymbolIdFragment &SF = cast<MCSymbolIdFragment>(F); const MCSymbolIdFragment &SF = cast<MCSymbolIdFragment>(F);
support::endian::write<uint32_t>(OS, SF.getSymbol()->getIndex(), Endian); support::endian::write<uint32_t>(OS, SF.getSymbol()->getIndex(), Endian);
break; break;
} }
case MCFragment::FT_Org: { case MCFragment::FT_Org: {
++stats::EmittedOrgFragments; ++stats::EmittedOrgFragments;
▲ Show 20 Lines • Show All 320 Lines • ▼ Show 20 Linesbool MCAssembler::relaxLEB(MCAsmLayout &Layout, MCLEBFragment &LF) {
// only increase an LEB fragment size here, not decrease it. See PR35809. // only increase an LEB fragment size here, not decrease it. See PR35809.
if (LF.isSigned()) if (LF.isSigned())
encodeSLEB128(Value, OSE, OldSize); encodeSLEB128(Value, OSE, OldSize);
else else
encodeULEB128(Value, OSE, OldSize); encodeULEB128(Value, OSE, OldSize);
return OldSize != LF.getContents().size(); return OldSize != LF.getContents().size();
} }
/// Check if the branch crosses the boundary.
///
/// \param StartAddr start address of the fused/unfused branch.
/// \param Size size of the fused/unfused branch.
/// \param BoundaryAlignment aligment requirement of the branch.
/// \returns true if the branch cross the boundary.
static bool mayCrossBoundary(uint64_t StartAddr, uint64_t Size,
Align BoundaryAlignment) {
uint64_t EndAddr = StartAddr + Size;
return (StartAddr >> Log2(BoundaryAlignment)) !=
((EndAddr - 1) >> Log2(BoundaryAlignment));
}
/// Check if the branch is against the boundary.
///
/// \param StartAddr start address of the fused/unfused branch.
/// \param Size size of the fused/unfused branch.
/// \param BoundaryAlignment aligment requirement of the branch.
/// \returns true if the branch is against the boundary.
static bool isAgainstBoundary(uint64_t StartAddr, uint64_t Size,
Align BoundaryAlignment) {
uint64_t EndAddr = StartAddr + Size;
MaskRayUnsubmitted
Not Done
ReplyInline Actions

Division is slow. Pass in the power of 2 and use right shift instead.

You may change MCMachineDependentFragment::AlignBoundarySize (a power of 2) to a power.

MaskRay: Division is slow. Pass in the power of 2 and use right shift instead. You may change…
return (EndAddr & (BoundaryAlignment.value() - 1)) == 0;
}
/// Check if the branch needs padding.
///
/// \param StartAddr start address of the fused/unfused branch.
/// \param Size size of the fused/unfused branch.
/// \param BoundaryAlignment aligment requirement of the branch.
MaskRayUnsubmitted
Done
ReplyInline Actions

Ditto.

MaskRay: Ditto.
/// \returns true if the branch needs padding.
static bool needPadding(uint64_t StartAddr, uint64_t Size,
Align BoundaryAlignment) {
return mayCrossBoundary(StartAddr, Size, BoundaryAlignment) ||
isAgainstBoundary(StartAddr, Size, BoundaryAlignment);
}
bool MCAssembler::relaxBoundaryAlign(MCAsmLayout &Layout,
MCBoundaryAlignFragment &BF) {
// The MCBoundaryAlignFragment that doesn't emit NOP should not be relaxed.
MaskRayUnsubmitted
Done
ReplyInline Actions

be relaxed

MaskRay: be relaxed
if (!BF.canEmitNops())
return false;
uint64_t AlignedOffset = Layout.getFragmentOffset(BF.getNextNode());
MaskRayUnsubmitted
Done
ReplyInline Actions

uint64_t

MaskRay: uint64_t
uint64_t AlignedSize = 0;
const MCFragment *F = BF.getNextNode();
MaskRayUnsubmitted
Done
ReplyInline Actions

auto -> MCFragment

MaskRay: `auto` -> `MCFragment`
// If the branch is unfused, it is emitted into one fragment, otherwise it is
// emitted into two fragments at most, the next MCBoundaryAlignFragment(if
// exists) also marks the end of the branch.
jyknightUnsubmitted
Done
ReplyInline Actions

I think this is broken -- moving symbols around like this risks causing evaluations of symbol offsets which may have already happened to be wrong.

Also it's ugly, and I can't tell why it's necessary, because there's no comments.

jyknight: I think this is broken -- moving symbols around like this risks causing evaluations of symbol…
for (auto i = 0, N = BF.isFused() ? 2 : 1;
MaskRayUnsubmitted
Not Done
ReplyInline Actions

unsigned -> int

MaskRay: unsigned -> int
skanAuthorUnsubmitted
Done
ReplyInline Actions

Why use int here?

skan: Why use `int` here?
MaskRayUnsubmitted
Not Done
ReplyInline Actions

The loop variable can only be 0 or 1. 0U 1U 2U the unsigned suffix are just redundant. int suffices. It also improves readability a bit by avoiding auto.

MaskRay: The loop variable can only be 0 or 1. 0U 1U 2U the unsigned suffix are just redundant. `int`…
i != N && !isa<MCBoundaryAlignFragment>(F); ++i, F = F->getNextNode()) {
AlignedSize += computeFragmentSize(Layout, *F);
}
uint64_t OldSize = BF.getSize();
MaskRayUnsubmitted
Done
ReplyInline Actions

uint64_t

MaskRay: uint64_t
AlignedOffset -= OldSize;
Align BoundaryAlignment = BF.getAlignment();
MaskRayUnsubmitted
Done
ReplyInline Actions

Align

MaskRay: `Align`
uint64_t NewSize = needPadding(AlignedOffset, AlignedSize, BoundaryAlignment)
? offsetToAlignment(AlignedOffset, BoundaryAlignment)
: 0U;
if (NewSize == OldSize)
return false;
BF.setSize(NewSize);
Layout.invalidateFragmentsFrom(&BF);
return true;
}
jyknightUnsubmitted
Not Done
ReplyInline Actions

I don't think this is necessary. AFAICT, the symbols should already be in the right place -- pointing to the relax fragment, not the instruction itself, without this. And removing all this moveSymbol/updateSymbolMap code doesn't make any tests fail.

jyknight: I don't think this is necessary. AFAICT, the symbols should already be in the right place…
skanAuthorUnsubmitted
Done
ReplyInline Actions

Yes, I check it and you are right. I will removing all this moveSymbol/updateSymbolMap code.

skan: Yes, I check it and you are right. I will removing all this moveSymbol/updateSymbolMap code.
bool MCAssembler::relaxDwarfLineAddr(MCAsmLayout &Layout, bool MCAssembler::relaxDwarfLineAddr(MCAsmLayout &Layout,
MCDwarfLineAddrFragment &DF) { MCDwarfLineAddrFragment &DF) {
MCContext &Context = Layout.getAssembler().getContext(); MCContext &Context = Layout.getAssembler().getContext();
uint64_t OldSize = DF.getContents().size(); uint64_t OldSize = DF.getContents().size();
int64_t AddrDelta; int64_t AddrDelta;
bool Abs = DF.getAddrDelta().evaluateKnownAbsolute(AddrDelta, Layout); bool Abs = DF.getAddrDelta().evaluateKnownAbsolute(AddrDelta, Layout);
assert(Abs && "We created a line delta with an invalid expression"); assert(Abs && "We created a line delta with an invalid expression");
(void)Abs; (void)Abs;
▲ Show 20 Lines • Show All 100 Lines • ▼ Show 20 Lines for (MCSection::iterator I = Sec.begin(), IE = Sec.end(); I != IE; ++I) {
case MCFragment::FT_DwarfFrame: case MCFragment::FT_DwarfFrame:
RelaxedFrag = RelaxedFrag =
relaxDwarfCallFrameFragment(Layout, relaxDwarfCallFrameFragment(Layout,
*cast<MCDwarfCallFrameFragment>(I)); *cast<MCDwarfCallFrameFragment>(I));
break; break;
case MCFragment::FT_LEB: case MCFragment::FT_LEB:
RelaxedFrag = relaxLEB(Layout, *cast<MCLEBFragment>(I)); RelaxedFrag = relaxLEB(Layout, *cast<MCLEBFragment>(I));
break; break;
case MCFragment::FT_BoundaryAlign:
RelaxedFrag =
relaxBoundaryAlign(Layout, *cast<MCBoundaryAlignFragment>(I));
break;
case MCFragment::FT_CVInlineLines: case MCFragment::FT_CVInlineLines:
RelaxedFrag = RelaxedFrag =
relaxCVInlineLineTable(Layout, *cast<MCCVInlineLineTableFragment>(I)); relaxCVInlineLineTable(Layout, *cast<MCCVInlineLineTableFragment>(I));
break; break;
case MCFragment::FT_CVDefRange: case MCFragment::FT_CVDefRange:
RelaxedFrag = relaxCVDefRange(Layout, *cast<MCCVDefRangeFragment>(I)); RelaxedFrag = relaxCVDefRange(Layout, *cast<MCCVDefRangeFragment>(I));
break; break;
} }
▲ Show 20 Lines • Show All 57 Lines • Show Last 20 Lines