[llvm] r174401 - [MC] Bundle alignment: Invalidate relaxed fragments

[David Blaikie]

On Tue, Feb 5, 2013 at 9:55 AM, Derek Schuff <dschuff at google.com> wrote:
> Author: dschuff
> Date: Tue Feb  5 11:55:27 2013
> New Revision: 174401
>
> URL: http://llvm.org/viewvc/llvm-project?rev=174401&view=rev
> Log:
> [MC] Bundle alignment: Invalidate relaxed fragments
>
> Currently, when a fragment is relaxed, its size is modified, but its
> offset is not (it gets laid out as a side effect of checking whether
> it needs relaxation), then all subsequent fragments are invalidated
> because their offsets need to change. When bundling is enabled,
> relaxed fragments need to get laid out again, because the increase in
> size may push it over a bundle boundary. So instead of only
> invalidating subsequent fragments, also invalidate the fragment that
> gets relaxed, which causes it to get laid out again.
>
> This patch also fixes some trailing whitespace and fixes the
> bundling-related debug output of MCFragments.
>
> Added:
>     llvm/trunk/test/MC/X86/AlignedBundling/relax-at-bundle-end.s   (with props)
> Modified:
>     llvm/trunk/include/llvm/MC/MCAsmLayout.h
>     llvm/trunk/lib/MC/MCAssembler.cpp
>
> Modified: llvm/trunk/include/llvm/MC/MCAsmLayout.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmLayout.h?rev=174401&r1=174400&r2=174401&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCAsmLayout.h (original)
> +++ llvm/trunk/include/llvm/MC/MCAsmLayout.h Tue Feb  5 11:55:27 2013
> @@ -60,9 +60,10 @@ public:
>    /// Get the assembler object this is a layout for.
>    MCAssembler &getAssembler() const { return Assembler; }
>
> -  /// \brief Invalidate the fragments after F because it has been resized.
> -  /// The fragment's size should have already been updated.
> -  void invalidateFragmentsAfter(MCFragment *F);
> +  /// \brief Invalidate the fragments starting with F because it has been
> +  /// resized. The fragment's size should have already been updated, but
> +  /// its bundle padding will be recomputed.
> +  void invalidateFragmentsFrom(MCFragment *F);
>
>    /// \brief Perform layout for a single fragment, assuming that the previous
>    /// fragment has already been laid out correctly, and the parent section has
>
> Modified: llvm/trunk/lib/MC/MCAssembler.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAssembler.cpp?rev=174401&r1=174400&r2=174401&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAssembler.cpp (original)
> +++ llvm/trunk/lib/MC/MCAssembler.cpp Tue Feb  5 11:55:27 2013
> @@ -82,14 +82,15 @@ bool MCAsmLayout::isFragmentValid(const
>    return F->getLayoutOrder() <= LastValid->getLayoutOrder();
>  }
>
> -void MCAsmLayout::invalidateFragmentsAfter(MCFragment *F) {
> +void MCAsmLayout::invalidateFragmentsFrom(MCFragment *F) {
>    // If this fragment wasn't already valid, we don't need to do anything.
>    if (!isFragmentValid(F))
>      return;
>
> -  // Otherwise, reset the last valid fragment to this fragment.
> +  // Otherwise, reset the last valid fragment to the previous fragment
> +  // (if this is the first fragment, it will be NULL).
>    const MCSectionData &SD = *F->getParent();
> -  LastValidFragment[&SD] = F;
> +  LastValidFragment[&SD] = F->getPrevNode();
>  }
>
>  void MCAsmLayout::ensureValid(const MCFragment *F) const {
> @@ -165,14 +166,14 @@ uint64_t MCAsmLayout::getSectionFileSize
>  uint64_t MCAsmLayout::computeBundlePadding(const MCFragment *F,
>                                             uint64_t FOffset, uint64_t FSize) {
>    uint64_t BundleSize = Assembler.getBundleAlignSize();
> -  assert(BundleSize > 0 &&
> +  assert(BundleSize > 0 &&
>           "computeBundlePadding should only be called if bundling is enabled");
>    uint64_t BundleMask = BundleSize - 1;
>    uint64_t OffsetInBundle = FOffset & BundleMask;
>    uint64_t EndOfFragment = OffsetInBundle + FSize;
>
>    // There are two kinds of bundling restrictions:
> -  //
> +  //
>    // 1) For alignToBundleEnd(), add padding to ensure that the fragment will
>    //    *end* on a bundle boundary.
>    // 2) Otherwise, check if the fragment would cross a bundle boundary. If it
> @@ -939,7 +940,7 @@ bool MCAssembler::layoutSectionOnce(MCAs
>        FirstRelaxedFragment = I;
>    }
>    if (FirstRelaxedFragment) {
> -    Layout.invalidateFragmentsAfter(FirstRelaxedFragment);
> +    Layout.invalidateFragmentsFrom(FirstRelaxedFragment);
>      return true;
>    }
>    return false;
> @@ -999,7 +1000,7 @@ void MCFragment::dump() {
>    OS << "<MCFragment " << (void*) this << " LayoutOrder:" << LayoutOrder
>       << " Offset:" << Offset
>       << " HasInstructions:" << hasInstructions()
> -     << " BundlePadding:" << getBundlePadding() << ">";
> +     << " BundlePadding:" << static_cast<unsigned>(getBundlePadding()) << ">";
>
>    switch (getKind()) {
>    case MCFragment::FT_Align: {
>
> Added: llvm/trunk/test/MC/X86/AlignedBundling/relax-at-bundle-end.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/AlignedBundling/relax-at-bundle-end.s?rev=174401&view=auto
> ==============================================================================
> --- llvm/trunk/test/MC/X86/AlignedBundling/relax-at-bundle-end.s (added)
> +++ llvm/trunk/test/MC/X86/AlignedBundling/relax-at-bundle-end.s Tue Feb  5 11:55:27 2013
> @@ -0,0 +1,16 @@
> +# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - \
> +# RUN:   | llvm-objdump -disassemble -no-show-raw-insn - | FileCheck %s
> +
> +# Test that an instruction near a bundle end gets properly padded
> +# after it is relaxed.
> +.text
> +foo:
> +        .bundle_align_mode 5
> +        .rept 29
> +        push %rax
> +        .endr
> +# CHECK: 1c: push
> +# CHECK: 1d: nop
> +# CHECK: 20: jne
> +        jne 0x100
> +
>
> Propchange: llvm/trunk/test/MC/X86/AlignedBundling/relax-at-bundle-end.s
> ------------------------------------------------------------------------------
>     svn:eol-style = LF

Not sure, but you might want to check your SVN config to have it add
files without a hardcoded eol-style. I think we just keep them in the
neutral eol-style generally, though I could be wrong.

- David