[llvm-commits] [llvm] r172572 - in /llvm/trunk: include/llvm/MC/MCAssembler.h lib/MC/MCAssembler.cpp lib/MC/MCELFStreamer.cpp

Evan Cheng evan.cheng at apple.com
Tue Jan 15 23:19:25 PST 2013


Hi Eli,

Is it possible to implement this without introducing virtual function like hasFixups()? I'm always suspicious of virtual functions that returns a simple query. Could the query simply check the fragment type?

Evan

On Jan 15, 2013, at 3:22 PM, Eli Bendersky <eliben at google.com> wrote:

> Author: eliben
> Date: Tue Jan 15 17:22:09 2013
> New Revision: 172572
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=172572&view=rev
> Log:
> Optimize the memory usage of MC bundling, by creating a new type of fragment
> into which we can emit single instructions without fixups (which is most
> instructions). This is an optimization required because MCDataFragment
> is prety large (240 bytes on x64), with no change in functionality.
> 
> For large programs, this reduces memory usage overhead required for bundling
> by 40%.
> 
> To make the code as palatable as possible, the MCEncodedFragment interface was
> further fragmented (no pun intended) and MCEncodedFragmentWithFixups is used
> as the interface to work against when the user expects fixups. MCDataFragment
> and MCRelaxableFragment implement this interface, while the new
> MCCompactEncodedInstFragment implements MCEncodeFragment.
> 
> 
> Modified:
>    llvm/trunk/include/llvm/MC/MCAssembler.h
>    llvm/trunk/lib/MC/MCAssembler.cpp
>    llvm/trunk/lib/MC/MCELFStreamer.cpp
> 
> Modified: llvm/trunk/include/llvm/MC/MCAssembler.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAssembler.h?rev=172572&r1=172571&r2=172572&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCAssembler.h (original)
> +++ llvm/trunk/include/llvm/MC/MCAssembler.h Tue Jan 15 17:22:09 2013
> @@ -47,6 +47,7 @@
>   enum FragmentType {
>     FT_Align,
>     FT_Data,
> +    FT_CompactEncodedInst,
>     FT_Fill,
>     FT_Relaxable,
>     FT_Org,
> @@ -105,6 +106,7 @@
> 
>   /// \brief Should this fragment be placed at the end of an aligned bundle?
>   virtual bool alignToBundleEnd() const { return false; }
> +  virtual void setAlignToBundleEnd(bool V) { }
> 
>   /// \brief Get the padding size that must be inserted before this fragment.
>   /// Used for bundling. By default, no padding is inserted.
> @@ -120,9 +122,16 @@
>   virtual void setBundlePadding(uint8_t N) {
>   }
> 
> +  virtual bool hasFixups() const {
> +    return false;
> +  }
> +
>   void dump();
> };
> 
> +/// Interface implemented by fragments that contain encoded instructions and/or
> +/// data.
> +///
> class MCEncodedFragment : public MCFragment {
>   virtual void anchor();
> 
> @@ -134,20 +143,9 @@
>   }
>   virtual ~MCEncodedFragment();
> 
> -  typedef SmallVectorImpl<MCFixup>::const_iterator const_fixup_iterator;
> -  typedef SmallVectorImpl<MCFixup>::iterator fixup_iterator;
> -
>   virtual SmallVectorImpl<char> &getContents() = 0;
>   virtual const SmallVectorImpl<char> &getContents() const = 0;
> 
> -  virtual SmallVectorImpl<MCFixup> &getFixups() = 0;
> -  virtual const SmallVectorImpl<MCFixup> &getFixups() const = 0;
> -
> -  virtual fixup_iterator fixup_begin() = 0;
> -  virtual const_fixup_iterator fixup_begin() const  = 0;
> -  virtual fixup_iterator fixup_end() = 0;
> -  virtual const_fixup_iterator fixup_end() const = 0;
> -
>   virtual uint8_t getBundlePadding() const {
>     return BundlePadding;
>   }
> @@ -158,13 +156,55 @@
> 
>   static bool classof(const MCFragment *F) {
>     MCFragment::FragmentType Kind = F->getKind();
> -    return Kind == MCFragment::FT_Relaxable || Kind == MCFragment::FT_Data;
> +    switch (Kind) {
> +      default:
> +        return false;
> +      case MCFragment::FT_Relaxable:
> +      case MCFragment::FT_CompactEncodedInst:
> +      case MCFragment::FT_Data:
> +        return true;
> +    }
> +  }
> +};
> +
> +/// Interface implemented by fragments that contain encoded instructions and/or
> +/// data and also have fixups registered.
> +///
> +class MCEncodedFragmentWithFixups : public MCEncodedFragment {
> +  virtual void anchor();
> +
> +public:
> +  MCEncodedFragmentWithFixups(MCFragment::FragmentType FType,
> +                              MCSectionData *SD = 0)
> +    : MCEncodedFragment(FType, SD)
> +  {
> +  }
> +
> +  virtual ~MCEncodedFragmentWithFixups();
> +
> +  virtual bool hasFixups() const {
> +    return true;
> +  }
> +
> +  typedef SmallVectorImpl<MCFixup>::const_iterator const_fixup_iterator;
> +  typedef SmallVectorImpl<MCFixup>::iterator fixup_iterator;
> +
> +  virtual SmallVectorImpl<MCFixup> &getFixups() = 0;
> +  virtual const SmallVectorImpl<MCFixup> &getFixups() const = 0;
> +
> +  virtual fixup_iterator fixup_begin() = 0;
> +  virtual const_fixup_iterator fixup_begin() const  = 0;
> +  virtual fixup_iterator fixup_end() = 0;
> +  virtual const_fixup_iterator fixup_end() const = 0;
> +
> +  static bool classof(const MCFragment *F) {
> +    return isa<MCEncodedFragment>(F) && F->hasFixups();
>   }
> };
> 
> /// Fragment for data and encoded instructions.
> ///
> -class MCDataFragment : public MCEncodedFragment {
> +class MCDataFragment : public MCEncodedFragmentWithFixups {
>   virtual void anchor();
> 
>   /// \brief Does this fragment contain encoded instructions anywhere in it?
> @@ -179,7 +219,7 @@
>   SmallVector<MCFixup, 4> Fixups;
> public:
>   MCDataFragment(MCSectionData *SD = 0)
> -    : MCEncodedFragment(FT_Data, SD),
> +    : MCEncodedFragmentWithFixups(FT_Data, SD),
>       HasInstructions(false), AlignToBundleEnd(false)
>   {
>   }
> @@ -212,10 +252,43 @@
>   }
> };
> 
> +/// This is a compact (memory-size-wise) fragment for holding an encoded
> +/// instruction (non-relaxable) that has no fixups registered. When applicable,
> +/// it can be used instead of MCDataFragment and lead to lower memory
> +/// consumption.
> +///
> +class MCCompactEncodedInstFragment : public MCEncodedFragment {
> +  virtual void anchor();
> +
> +  /// \brief Should this fragment be aligned to the end of a bundle?
> +  bool AlignToBundleEnd;
> +
> +  SmallVector<char, 4> Contents;
> +public:
> +  MCCompactEncodedInstFragment(MCSectionData *SD = 0)
> +    : MCEncodedFragment(FT_CompactEncodedInst, SD), AlignToBundleEnd(false)
> +  {
> +  }
> +
> +  virtual bool hasInstructions() const {
> +    return true;
> +  }
> +
> +  virtual SmallVectorImpl<char> &getContents() { return Contents; }
> +  virtual const SmallVectorImpl<char> &getContents() const { return Contents; }
> +
> +  virtual bool alignToBundleEnd() const { return AlignToBundleEnd; }
> +  virtual void setAlignToBundleEnd(bool V) { AlignToBundleEnd = V; }
> +
> +  static bool classof(const MCFragment *F) {
> +    return F->getKind() == MCFragment::FT_CompactEncodedInst;
> +  }
> +};
> +
> /// A relaxable fragment holds on to its MCInst, since it may need to be
> /// relaxed during the assembler layout and relaxation stage.
> ///
> -class MCRelaxableFragment : public MCEncodedFragment {
> +class MCRelaxableFragment : public MCEncodedFragmentWithFixups {
>   virtual void anchor();
> 
>   /// Inst - The instruction this is a fragment for.
> @@ -229,7 +302,7 @@
> 
> public:
>   MCRelaxableFragment(const MCInst &_Inst, MCSectionData *SD = 0)
> -    : MCEncodedFragment(FT_Relaxable, SD), Inst(_Inst) {
> +    : MCEncodedFragmentWithFixups(FT_Relaxable, SD), Inst(_Inst) {
>   }
> 
>   virtual SmallVectorImpl<char> &getContents() { return Contents; }
> 
> Modified: llvm/trunk/lib/MC/MCAssembler.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAssembler.cpp?rev=172572&r1=172571&r2=172572&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAssembler.cpp (original)
> +++ llvm/trunk/lib/MC/MCAssembler.cpp Tue Jan 15 17:22:09 2013
> @@ -38,6 +38,8 @@
>           "Number of emitted assembler fragments - relaxable");
> STATISTIC(EmittedDataFragments,
>           "Number of emitted assembler fragments - data");
> +STATISTIC(EmittedCompactEncodedInstFragments,
> +          "Number of emitted assembler fragments - compact encoded inst");
> STATISTIC(EmittedAlignFragments,
>           "Number of emitted assembler fragments - align");
> STATISTIC(EmittedFillFragments,
> @@ -222,6 +224,11 @@
> 
> /* *** */
> 
> +MCEncodedFragmentWithFixups::~MCEncodedFragmentWithFixups() {
> +}
> +
> +/* *** */
> +
> MCSectionData::MCSectionData() : Section(0) {}
> 
> MCSectionData::MCSectionData(const MCSection &_Section, MCAssembler *A)
> @@ -388,6 +395,7 @@
>   switch (F.getKind()) {
>   case MCFragment::FT_Data:
>   case MCFragment::FT_Relaxable:
> +  case MCFragment::FT_CompactEncodedInst:
>     return cast<MCEncodedFragment>(F).getContents().size();
>   case MCFragment::FT_Fill:
>     return cast<MCFillFragment>(F).getSize();
> @@ -570,6 +578,11 @@
>     writeFragmentContents(F, OW);
>     break;
> 
> +  case MCFragment::FT_CompactEncodedInst:
> +    ++stats::EmittedCompactEncodedInstFragments;
> +    writeFragmentContents(F, OW);
> +    break;
> +
>   case MCFragment::FT_Fill: {
>     ++stats::EmittedFillFragments;
>     MCFillFragment &FF = cast<MCFillFragment>(F);
> @@ -742,9 +755,10 @@
>   for (MCAssembler::iterator it = begin(), ie = end(); it != ie; ++it) {
>     for (MCSectionData::iterator it2 = it->begin(),
>            ie2 = it->end(); it2 != ie2; ++it2) {
> -      MCEncodedFragment *F = dyn_cast<MCEncodedFragment>(it2);
> +      MCEncodedFragmentWithFixups *F =
> +        dyn_cast<MCEncodedFragmentWithFixups>(it2);
>       if (F) {
> -        for (MCEncodedFragment::fixup_iterator it3 = F->fixup_begin(),
> +        for (MCEncodedFragmentWithFixups::fixup_iterator it3 = F->fixup_begin(),
>              ie3 = F->fixup_end(); it3 != ie3; ++it3) {
>           MCFixup &Fixup = *it3;
>           uint64_t FixedValue = handleFixup(Layout, *F, Fixup);
> @@ -954,6 +968,8 @@
>   switch (getKind()) {
>   case MCFragment::FT_Align: OS << "MCAlignFragment"; break;
>   case MCFragment::FT_Data:  OS << "MCDataFragment"; break;
> +  case MCFragment::FT_CompactEncodedInst:
> +    OS << "MCCompactEncodedInstFragment"; break;
>   case MCFragment::FT_Fill:  OS << "MCFillFragment"; break;
>   case MCFragment::FT_Relaxable:  OS << "MCRelaxableFragment"; break;
>   case MCFragment::FT_Org:   OS << "MCOrgFragment"; break;
> @@ -1001,6 +1017,19 @@
>     }
>     break;
>   }
> +  case MCFragment::FT_CompactEncodedInst: {
> +    const MCCompactEncodedInstFragment *CEIF =
> +      cast<MCCompactEncodedInstFragment>(this);
> +    OS << "\n       ";
> +    OS << " Contents:[";
> +    const SmallVectorImpl<char> &Contents = CEIF->getContents();
> +    for (unsigned i = 0, e = Contents.size(); i != e; ++i) {
> +      if (i) OS << ",";
> +      OS << hexdigit((Contents[i] >> 4) & 0xF) << hexdigit(Contents[i] & 0xF);
> +    }
> +    OS << "] (" << Contents.size() << " bytes)";
> +    break;
> +  }
>   case MCFragment::FT_Fill:  {
>     const MCFillFragment *FF = cast<MCFillFragment>(this);
>     OS << " Value:" << FF->getValue() << " ValueSize:" << FF->getValueSize()
> @@ -1094,7 +1123,9 @@
> 
> // anchors for MC*Fragment vtables
> void MCEncodedFragment::anchor() { }
> +void MCEncodedFragmentWithFixups::anchor() { }
> void MCDataFragment::anchor() { }
> +void MCCompactEncodedInstFragment::anchor() { }
> void MCRelaxableFragment::anchor() { }
> void MCAlignFragment::anchor() { }
> void MCFillFragment::anchor() { }
> @@ -1102,3 +1133,4 @@
> void MCLEBFragment::anchor() { }
> void MCDwarfLineAddrFragment::anchor() { }
> void MCDwarfCallFrameFragment::anchor() { }
> +
> 
> Modified: llvm/trunk/lib/MC/MCELFStreamer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCELFStreamer.cpp?rev=172572&r1=172571&r2=172572&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCELFStreamer.cpp (original)
> +++ llvm/trunk/lib/MC/MCELFStreamer.cpp Tue Jan 15 17:22:09 2013
> @@ -371,8 +371,10 @@
>   // data fragment).
>   //
>   // If bundling is enabled:
> -  // - If we're not in a bundle-locked group, emit the instruction into a data
> -  //   fragment of its own.
> +  // - If we're not in a bundle-locked group, emit the instruction into a
> +  //   fragment of its own. If there are no fixups registered for the
> +  //   instruction, emit a MCCompactEncodedInstFragment. Otherwise, emit a
> +  //   MCDataFragment.
>   // - If we're in a bundle-locked group, append the instruction to the current
>   //   data fragment because we want all the instructions in a group to get into
>   //   the same fragment. Be careful not to do that for the first instruction in
> @@ -383,6 +385,14 @@
>     MCSectionData *SD = getCurrentSectionData();
>     if (SD->isBundleLocked() && !SD->isBundleGroupBeforeFirstInst())
>       DF = getOrCreateDataFragment();
> +    else if (!SD->isBundleLocked() && Fixups.size() == 0) {
> +      // Optimize memory usage by emitting the instruction to a
> +      // MCCompactEncodedInstFragment when not in a bundle-locked group and
> +      // there are no fixups registered.
> +      MCCompactEncodedInstFragment *CEIF = new MCCompactEncodedInstFragment(SD);
> +      CEIF->getContents().append(Code.begin(), Code.end());
> +      return;
> +    }
>     else {
>       DF = new MCDataFragment(SD);
>       if (SD->getBundleLockState() == MCSectionData::BundleLockedAlignToEnd) {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list