diff --git a/llvm/include/llvm/MC/MCFragment.h b/llvm/include/llvm/MC/MCFragment.h --- a/llvm/include/llvm/MC/MCFragment.h +++ b/llvm/include/llvm/MC/MCFragment.h @@ -227,10 +227,17 @@ /// Fragment for data and encoded instructions. /// class MCDataFragment : public MCEncodedFragmentWithFixups<32, 4> { + /// Flag to indicate that further data are allowed to be emitted into the data + /// fragment. + bool Reuse = true; + public: MCDataFragment(MCSection *Sec = nullptr) : MCEncodedFragmentWithFixups<32, 4>(FT_Data, false, Sec) {} + bool getReuse() const { return Reuse; } + void setReuse(bool V) { Reuse = V; } + static bool classof(const MCFragment *F) { return F->getKind() == MCFragment::FT_Data; } @@ -523,26 +530,24 @@ class MCBoundaryAlignFragment : public MCFragment { /// The alignment requirement of the branch to be aligned. Align AlignBoundary; - /// Flag to indicate whether the branch is fused. Use in determining the - /// region of fragments being aligned. - bool Fused : 1; - /// Flag to indicate whether NOPs should be emitted. - bool EmitNops : 1; + /// The last fragment in the set of fragments to be aligned. + const MCFragment *LastFragment = nullptr; public: - MCBoundaryAlignFragment(Align AlignBoundary = Align(1), bool Fused = false, - bool EmitNops = false, MCSection *Sec = nullptr) - : MCFragment(FT_BoundaryAlign, false, Sec), AlignBoundary(AlignBoundary), - Fused(Fused), EmitNops(EmitNops) {} + MCBoundaryAlignFragment(Align AlignBoundary, MCSection *Sec = nullptr) + : MCFragment(FT_BoundaryAlign, false, Sec), AlignBoundary(AlignBoundary) { + } Align getAlignment() const { return AlignBoundary; } void setAlignment(Align Value) { AlignBoundary = Value; } - bool isFused() const { return Fused; } - void setFused(bool Value) { Fused = Value; } - - bool canEmitNops() const { return EmitNops; } - void setEmitNops(bool Value) { EmitNops = Value; } + const MCFragment *getLastFragment() const { return LastFragment; } + void setLastFragment(const MCFragment *F) { + assert(getParent() == F->getParent() && + "The fragments to be aligned must follow " + "this one within same section."); + LastFragment = F; + } static bool classof(const MCFragment *F) { return F->getKind() == MCFragment::FT_BoundaryAlign; diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp --- a/llvm/lib/MC/MCAssembler.cpp +++ b/llvm/lib/MC/MCAssembler.cpp @@ -353,18 +353,15 @@ case MCFragment::FT_BoundaryAlign: { const MCBoundaryAlignFragment &BF = cast(F); - // MCBoundaryAlignFragment that doesn't emit NOP should have 0 size. - if (!BF.canEmitNops()) + // MCBoundaryAlignFragment that doesn't need to align any fragment + // should have 0 size. + if (!BF.getLastFragment()) return 0; uint64_t AlignedOffset = Layout.getFragmentOffset(&BF); uint64_t AlignedSize = 0; - const MCFragment *F = BF.getNextNode(); - // 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. - for (int I = 0, N = BF.isFused() ? 2 : 1; - I != N && !isa(F); ++I, F = F->getNextNode()) + for (const MCFragment *F = BF.getLastFragment(); F != &BF; + F = F->getPrevNode()) AlignedSize += computeFragmentSize(Layout, *F); Align BoundaryAlignment = BF.getAlignment(); return needPadding(AlignedOffset, AlignedSize, BoundaryAlignment) diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp --- a/llvm/lib/MC/MCFragment.cpp +++ b/llvm/lib/MC/MCFragment.cpp @@ -424,14 +424,9 @@ } case MCFragment::FT_BoundaryAlign: { const auto *BF = cast(this); - if (BF->canEmitNops()) - OS << " (can emit nops to align"; - if (BF->isFused()) - OS << " fused branch)"; - else - OS << " unfused branch)"; OS << "\n "; OS << " BoundarySize:" << BF->getAlignment().value(); + OS << " LastFragment:" << BF->getLastFragment(); break; } case MCFragment::FT_SymbolId: { diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp --- a/llvm/lib/MC/MCObjectStreamer.cpp +++ b/llvm/lib/MC/MCObjectStreamer.cpp @@ -194,6 +194,10 @@ static bool CanReuseDataFragment(const MCDataFragment &F, const MCAssembler &Assembler, const MCSubtargetInfo *STI) { + // When the data fragment is explictly set to be unreusable, we start a new + // fragment. + if (!F.getReuse()) + return false; if (!F.hasInstructions()) return true; // When bundling is enabled, we don't want to add data to a fragment that @@ -215,15 +219,6 @@ return F; } -MCBoundaryAlignFragment *MCObjectStreamer::getOrCreateBoundaryAlignFragment() { - auto *F = dyn_cast_or_null(getCurrentFragment()); - if (!F || F->canEmitNops()) { - F = new MCBoundaryAlignFragment(); - insert(F); - } - return F; -} - void MCObjectStreamer::visitUsedSymbol(const MCSymbol &Sym) { Assembler->registerSymbol(Sym); } diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp --- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp +++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp @@ -121,6 +121,7 @@ bool needAlign(MCObjectStreamer &OS) const; bool needAlignInst(const MCInst &Inst) const; MCInst PrevInst; + MCBoundaryAlignFragment *PendingBoundaryAlign = nullptr; public: X86AsmBackend(const Target &T, const MCSubtargetInfo &STI) @@ -393,25 +394,27 @@ (AlignBranchType & X86::AlignBranchIndirect)); } -/// Insert MCBoundaryAlignFragment before instructions to align branches. +/// Insert BoundaryAlignFragment before instructions to align branches. void X86AsmBackend::alignBranchesBegin(MCObjectStreamer &OS, const MCInst &Inst) { if (!needAlign(OS)) return; + bool IsMacroFused = isMacroFused(PrevInst, Inst); + // Macor fusion doesn't happen indeed, clear the pending. + if (!IsMacroFused) + PendingBoundaryAlign = nullptr; + MCFragment *CF = OS.getCurrentFragment(); bool NeedAlignFused = AlignBranchType & X86::AlignBranchFused; - if (NeedAlignFused && isMacroFused(PrevInst, Inst) && CF) { + if (PendingBoundaryAlign && CF->getPrevNode() == PendingBoundaryAlign) // Macro fusion actually happens and there is no other fragment inserted - // after the previous instruction. NOP can be emitted in PF to align fused - // jcc. - if (auto *PF = - dyn_cast_or_null(CF->getPrevNode())) { - const_cast(PF)->setEmitNops(true); - const_cast(PF)->setFused(true); - } - } else if (needAlignInst(Inst)) { - // Note: When there is at least one fragment, such as MCAlignFragment, + // after the previous instruction. + // + // Do nothing here since we already inserted a BoudaryAlign fragment when + // we met the first instruction in the fused pair. + // + // Note: When there is at least one fragment, such as AlignFragment, // inserted after the previous instruction, e.g. // // \code @@ -422,36 +425,36 @@ // // We will treat the JCC as a unfused branch although it may be fused // with the CMP. - auto *F = OS.getOrCreateBoundaryAlignFragment(); - F->setAlignment(AlignBoundary); - F->setEmitNops(true); - F->setFused(false); - } else if (NeedAlignFused && isFirstMacroFusibleInst(Inst, *MCII)) { - // We don't know if macro fusion happens until the reaching the next - // instruction, so a place holder is put here if necessary. - auto *F = OS.getOrCreateBoundaryAlignFragment(); - F->setAlignment(AlignBoundary); - } - - PrevInst = Inst; + return; + else if (needAlignInst(Inst) || + (NeedAlignFused && isFirstMacroFusibleInst(Inst, *MCII))) + // If we meet a unfused branch or the first instuction in a fusiable pair, + // insert a BoundaryAlign fragment. + OS.insert(PendingBoundaryAlign = + new MCBoundaryAlignFragment(AlignBoundary)); } -/// Insert a MCBoundaryAlignFragment to mark the end of the branch to be aligned -/// if necessary. +/// Set the last fragment to be aligned for the BoundaryAlignFragment. void X86AsmBackend::alignBranchesEnd(MCObjectStreamer &OS, const MCInst &Inst) { if (!needAlign(OS)) return; - // If the branch is emitted into a MCRelaxableFragment, we can determine the - // size of the branch easily in during the process of layout. When the - // branch is fused, the fused branch(macro fusion pair) must be emitted into - // two fragments. Or when the branch is unfused, the branch must be emitted - // into one fragment. The MCRelaxableFragment naturally marks the end of the - // fused or unfused branch. - // Otherwise, we need to insert a MCBoundaryAlignFragment to mark the end of - // the branch. This MCBoundaryAlignFragment may be reused to emit NOP to align - // other branch. - if (needAlignInst(Inst) && !isa(OS.getCurrentFragment())) - OS.insert(new MCBoundaryAlignFragment(AlignBoundary)); + + PrevInst = Inst; + + if (!needAlignInst(Inst)) + return; + + MCFragment *CF = OS.getCurrentFragment(); + if (PendingBoundaryAlign) { + PendingBoundaryAlign->setLastFragment(CF); + } + + PendingBoundaryAlign = nullptr; + + // We need to ensure that further data aren't added to the current data + // fragment. + if (auto *DF = dyn_cast_or_null(CF)) + DF->setReuse(false); // Update the maximum alignment on the current section if necessary. MCSection *Sec = OS.getCurrentSectionOnly(); diff --git a/llvm/test/MC/X86/align-branch-64-negative.s b/llvm/test/MC/X86/align-branch-64-negative.s --- a/llvm/test/MC/X86/align-branch-64-negative.s +++ b/llvm/test/MC/X86/align-branch-64-negative.s @@ -27,11 +27,13 @@ # In the second test, we have a label which is expected to be bound to the # end of the call. For instance, we want the to associate some data with # the return address of the call. - # CHECK: labeled_call_test2: - # CHECK: 5a: callq - # CHECK: 5f: nop - # CHECK: 60 label_after - # CHECK: 60: jmp +## # check: labeled_call_test2: +## # check: 5a: callq +## # check: 5f: nop +## # check: 60 label_after +## # check: 60: jmp +## The label "label_after" will be at address 0x5f now, should we remove second +## test? .globl labeled_call_test2 .p2align 5 labeled_call_test2: