[MC] Compute fragment offsets eagerly

This builds on top of commit 9d0754a
("[MC] Relax fragments eagerly") and relaxes fragments eagerly to
eliminate MCSection::HasLayout and `getFragmentOffset` overhead. The
approach is slightly different from
1a47f3f and has less performance
benefit.

The new layout algorithm also addresses the following problems:

* Size change of MCFillFragment/MCOrgFragment did not influence the
  fixed-point iteration, which could be problematic for contrived cases.
* The `invalid number of bytes` error was reported too early. Since
  `.zero A-B` might have temporary negative values in the first few
  iterations.
* X86AsmBackend::finishLayout performed only one iteration, which might
  not converge. In addition, the removed `#ifndef NDEBUG` code (disabled
  by default) in X86AsmBackend::finishLayout was problematic, as !NDEBUG
  and NDEBUG builds evaluated fragment offsets at different times before
  this patch.
* The computed layout for relax-recompute-align.s is optimal now.

Builds with many text sections (e.g. full LTO) shall observe a decrease
in compile time while the new algorithm could be slightly slower for
some -O0 -g projects.

Aligned bundling from the deprecated PNaCl placed constraints how we can
perform iteration.

Author: MaskRay

llvm/include/llvm/MC/MCAsmBackend.h

@@ -217,8 +217,9 @@ class MCAsmBackend {
   virtual bool writeNopData(raw_ostream &OS, uint64_t Count,
                             const MCSubtargetInfo *STI) const = 0;
 
-  /// Give backend an opportunity to finish layout after relaxation
-  virtual void finishLayout(MCAssembler const &Asm) const {}
+  // Return true if fragment offsets have been adjusted and an extra layout
+  // iteration is needed.
+  virtual bool finishLayout(const MCAssembler &Asm) const { return false; }
 
   /// Handle any target-specific assembler flags. By default, do nothing.
   virtual void handleAssemblerFlag(MCAssemblerFlag Flag) {}

llvm/include/llvm/MC/MCAssembler.h

@@ -65,6 +65,7 @@ class MCAssembler {
 
   bool HasLayout = false;
   bool RelaxAll = false;
+  unsigned RelaxSteps = 0;
 
   SectionListType Sections;
 
@@ -113,19 +114,18 @@ class MCAssembler {
 
   /// Perform one layout iteration and return true if any offsets
   /// were adjusted.
-  bool layoutOnce();
-
-  /// Perform relaxation on a single fragment - returns true if the fragment
-  /// changes as a result of relaxation.
-  bool relaxFragment(MCFragment &F);
-  bool relaxInstruction(MCRelaxableFragment &IF);
-  bool relaxLEB(MCLEBFragment &IF);
-  bool relaxBoundaryAlign(MCBoundaryAlignFragment &BF);
-  bool relaxDwarfLineAddr(MCDwarfLineAddrFragment &DF);
-  bool relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF);
-  bool relaxCVInlineLineTable(MCCVInlineLineTableFragment &DF);
-  bool relaxCVDefRange(MCCVDefRangeFragment &DF);
-  bool relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &DF);
+  bool relaxOnce();
+
+  /// Perform relaxation on a single fragment.
+  void relaxFragment(MCFragment &F);
+  void relaxInstruction(MCRelaxableFragment &IF);
+  void relaxLEB(MCLEBFragment &IF);
+  void relaxBoundaryAlign(MCBoundaryAlignFragment &BF);
+  void relaxDwarfLineAddr(MCDwarfLineAddrFragment &DF);
+  void relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF);
+  void relaxCVInlineLineTable(MCCVInlineLineTableFragment &DF);
+  void relaxCVDefRange(MCCVDefRangeFragment &DF);
+  void relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &DF);
 
   std::tuple<MCValue, uint64_t, bool>
   handleFixup(MCFragment &F, const MCFixup &Fixup, const MCSubtargetInfo *STI);
@@ -147,10 +147,9 @@ class MCAssembler {
   uint64_t computeFragmentSize(const MCFragment &F) const;
 
   void layoutBundle(MCFragment *Prev, MCFragment *F) const;
-  void ensureValid(MCSection &Sec) const;
 
   // Get the offset of the given fragment inside its containing section.
-  uint64_t getFragmentOffset(const MCFragment &F) const;
+  uint64_t getFragmentOffset(const MCFragment &F) const { return F.Offset; }
 
   uint64_t getSectionAddressSize(const MCSection &Sec) const;
   uint64_t getSectionFileSize(const MCSection &Sec) const;

llvm/include/llvm/MC/MCSection.h

@@ -99,8 +99,6 @@ class MCSection {
   /// Whether this section has had instructions emitted into it.
   bool HasInstructions : 1;
 
-  bool HasLayout : 1;
-
   bool IsRegistered : 1;
 
   bool IsText : 1;
@@ -169,9 +167,6 @@ class MCSection {
   bool hasInstructions() const { return HasInstructions; }
   void setHasInstructions(bool Value) { HasInstructions = Value; }
 
-  bool hasLayout() const { return HasLayout; }
-  void setHasLayout(bool Value) { HasLayout = Value; }
-
   bool isRegistered() const { return IsRegistered; }
   void setIsRegistered(bool Value) { IsRegistered = Value; }
 

llvm/lib/MC/MCAssembler.cpp

@@ -261,7 +261,11 @@ uint64_t MCAssembler::computeFragmentSize(const MCFragment &F) const {
     }
     int64_t Size = NumValues * FF.getValueSize();
     if (Size < 0) {
-      getContext().reportError(FF.getLoc(), "invalid number of bytes");
+      // The expression might use symbol values which have not yet converged.
+      // Allow the first few iterations to have temporary negative values. The
+      // limit is somewhat arbitrary but allows contrived interdependency.
+      if (RelaxSteps >= 2)
+        getContext().reportError(FF.getLoc(), "invalid number of bytes");
       return 0;
     }
     return Size;
@@ -428,28 +432,6 @@ void MCAssembler::layoutBundle(MCFragment *Prev, MCFragment *F) const {
       DF->Offset = EF->Offset;
 }
 
-void MCAssembler::ensureValid(MCSection &Sec) const {
-  if (Sec.hasLayout())
-    return;
-  Sec.setHasLayout(true);
-  MCFragment *Prev = nullptr;
-  uint64_t Offset = 0;
-  for (MCFragment &F : Sec) {
-    F.Offset = Offset;
-    if (isBundlingEnabled() && F.hasInstructions()) {
-      layoutBundle(Prev, &F);
-      Offset = F.Offset;
-    }
-    Offset += computeFragmentSize(F);
-    Prev = &F;
-  }
-}
-
-uint64_t MCAssembler::getFragmentOffset(const MCFragment &F) const {
-  ensureValid(*F.getParent());
-  return F.Offset;
-}
-
 // Simple getSymbolOffset helper for the non-variable case.
 static bool getLabelOffset(const MCAssembler &Asm, const MCSymbol &S,
                            bool ReportError, uint64_t &Val) {
@@ -929,22 +911,38 @@ void MCAssembler::layout() {
 
   // Layout until everything fits.
   this->HasLayout = true;
-  while (layoutOnce()) {
+  for (MCSection &Sec : *this) {
+    MCFragment *Prev = nullptr;
+    uint64_t Offset = 0;
+    for (MCFragment &F : Sec) {
+      F.Offset = Offset;
+      if (LLVM_UNLIKELY(isBundlingEnabled())) {
+        if (F.hasInstructions()) {
+          layoutBundle(Prev, &F);
+          Offset = F.Offset;
+        }
+        Prev = &F;
+      }
+      Offset += computeFragmentSize(F);
+    }
+  }
+  while (relaxOnce())
     if (getContext().hadError())
       return;
-    // Size of fragments in one section can depend on the size of fragments in
-    // another. If any fragment has changed size, we have to re-layout (and
-    // as a result possibly further relax) all.
-    for (MCSection &Sec : *this)
-      Sec.setHasLayout(false);
-  }
 
   DEBUG_WITH_TYPE("mc-dump", {
       errs() << "assembler backend - post-relaxation\n--\n";
       dump(); });
 
-  // Finalize the layout, including fragment lowering.
-  getBackend().finishLayout(*this);
+  // Some targets might want to adjust fragment offsets. If so, perform another
+  // relaxation loop.
+  if (getBackend().finishLayout(*this))
+    while (relaxOnce())
+      if (getContext().hadError())
+        return;
+
+  // Trigger computeFragmentSize errors.
+  RelaxSteps = UINT_MAX;
 
   DEBUG_WITH_TYPE("mc-dump", {
       errs() << "assembler backend - final-layout\n--\n";
@@ -1073,11 +1071,11 @@ bool MCAssembler::fragmentNeedsRelaxation(const MCRelaxableFragment *F) const {
   return false;
 }
 
-bool MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
+void MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
   assert(getEmitterPtr() &&
          "Expected CodeEmitter defined for relaxInstruction");
   if (!fragmentNeedsRelaxation(&F))
-    return false;
+    return;
 
   ++stats::RelaxedInstructions;
 
@@ -1095,10 +1093,9 @@ bool MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
   F.getContents().clear();
   getEmitter().encodeInstruction(Relaxed, F.getContents(), F.getFixups(),
                                  *F.getSubtargetInfo());
-  return true;
 }
 
-bool MCAssembler::relaxLEB(MCLEBFragment &LF) {
+void MCAssembler::relaxLEB(MCLEBFragment &LF) {
   const unsigned OldSize = static_cast<unsigned>(LF.getContents().size());
   unsigned PadTo = OldSize;
   int64_t Value;
@@ -1134,7 +1131,6 @@ bool MCAssembler::relaxLEB(MCLEBFragment &LF) {
     encodeSLEB128(Value, OSE, PadTo);
   else
     encodeULEB128(Value, OSE, PadTo);
-  return OldSize != LF.getContents().size();
 }
 
 /// Check if the branch crosses the boundary.
@@ -1174,11 +1170,11 @@ static bool needPadding(uint64_t StartAddr, uint64_t Size,
          isAgainstBoundary(StartAddr, Size, BoundaryAlignment);
 }
 
-bool MCAssembler::relaxBoundaryAlign(MCBoundaryAlignFragment &BF) {
+void MCAssembler::relaxBoundaryAlign(MCBoundaryAlignFragment &BF) {
   // BoundaryAlignFragment that doesn't need to align any fragment should not be
   // relaxed.
   if (!BF.getLastFragment())
-    return false;
+    return;
 
   uint64_t AlignedOffset = getFragmentOffset(BF);
   uint64_t AlignedSize = 0;
@@ -1192,19 +1188,15 @@ bool MCAssembler::relaxBoundaryAlign(MCBoundaryAlignFragment &BF) {
   uint64_t NewSize = needPadding(AlignedOffset, AlignedSize, BoundaryAlignment)
                          ? offsetToAlignment(AlignedOffset, BoundaryAlignment)
                          : 0U;
-  if (NewSize == BF.getSize())
-    return false;
   BF.setSize(NewSize);
-  return true;
 }
 
-bool MCAssembler::relaxDwarfLineAddr(MCDwarfLineAddrFragment &DF) {
+void MCAssembler::relaxDwarfLineAddr(MCDwarfLineAddrFragment &DF) {
   bool WasRelaxed;
   if (getBackend().relaxDwarfLineAddr(*this, DF, WasRelaxed))
-    return WasRelaxed;
+    return;
 
   MCContext &Context = getContext();
-  uint64_t OldSize = DF.getContents().size();
   int64_t AddrDelta;
   bool Abs = DF.getAddrDelta().evaluateKnownAbsolute(AddrDelta, *this);
   assert(Abs && "We created a line delta with an invalid expression");
@@ -1217,13 +1209,12 @@ bool MCAssembler::relaxDwarfLineAddr(MCDwarfLineAddrFragment &DF) {
 
   MCDwarfLineAddr::encode(Context, getDWARFLinetableParams(), LineDelta,
                           AddrDelta, Data);
-  return OldSize != Data.size();
 }
 
-bool MCAssembler::relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF) {
+void MCAssembler::relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF) {
   bool WasRelaxed;
   if (getBackend().relaxDwarfCFA(*this, DF, WasRelaxed))
-    return WasRelaxed;
+    return;
 
   MCContext &Context = getContext();
   int64_t Value;
@@ -1232,31 +1223,25 @@ bool MCAssembler::relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF) {
     getContext().reportError(DF.getAddrDelta().getLoc(),
                              "invalid CFI advance_loc expression");
     DF.setAddrDelta(MCConstantExpr::create(0, Context));
-    return false;
+    return;
   }
 
   SmallVectorImpl<char> &Data = DF.getContents();
-  uint64_t OldSize = Data.size();
   Data.clear();
   DF.getFixups().clear();
 
   MCDwarfFrameEmitter::encodeAdvanceLoc(Context, Value, Data);
-  return OldSize != Data.size();
 }
 
-bool MCAssembler::relaxCVInlineLineTable(MCCVInlineLineTableFragment &F) {
-  unsigned OldSize = F.getContents().size();
+void MCAssembler::relaxCVInlineLineTable(MCCVInlineLineTableFragment &F) {
   getContext().getCVContext().encodeInlineLineTable(*this, F);
-  return OldSize != F.getContents().size();
 }
 
-bool MCAssembler::relaxCVDefRange(MCCVDefRangeFragment &F) {
-  unsigned OldSize = F.getContents().size();
+void MCAssembler::relaxCVDefRange(MCCVDefRangeFragment &F) {
   getContext().getCVContext().encodeDefRange(*this, F);
-  return OldSize != F.getContents().size();
 }
 
-bool MCAssembler::relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &PF) {
+void MCAssembler::relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &PF) {
   uint64_t OldSize = PF.getContents().size();
   int64_t AddrDelta;
   bool Abs = PF.getAddrDelta().evaluateKnownAbsolute(AddrDelta, *this);
@@ -1269,13 +1254,12 @@ bool MCAssembler::relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &PF) {
 
   // AddrDelta is a signed integer
   encodeSLEB128(AddrDelta, OSE, OldSize);
-  return OldSize != Data.size();
 }
 
-bool MCAssembler::relaxFragment(MCFragment &F) {
+void MCAssembler::relaxFragment(MCFragment &F) {
   switch(F.getKind()) {
   default:
-    return false;
+    return;
   case MCFragment::FT_Relaxable:
     assert(!getRelaxAll() &&
            "Did not expect a MCRelaxableFragment in RelaxAll mode");
@@ -1297,15 +1281,57 @@ bool MCAssembler::relaxFragment(MCFragment &F) {
   }
 }
 
-bool MCAssembler::layoutOnce() {
+bool MCAssembler::relaxOnce() {
   ++stats::RelaxationSteps;
+  ++RelaxSteps;
 
-  bool Changed = false;
-  for (MCSection &Sec : *this)
-    for (MCFragment &Frag : Sec)
-      if (relaxFragment(Frag))
-        Changed = true;
-  return Changed;
+  // Size of fragments in one section can depend on the size of fragments in
+  // another. If any fragment has changed size, we have to re-layout (and
+  // as a result possibly further relax) all sections.
+  bool ChangedAny = false, Changed;
+  for (MCSection &Sec : *this) {
+    // Assume each iteration finalizes at least one extra fragment. If the
+    // layout does not converge after N+1 iterations, bail out.
+    auto MaxIter = Sec.curFragList()->Tail->getLayoutOrder() + 1;
+    uint64_t OldSize = getSectionAddressSize(Sec);
+    do {
+      uint64_t Offset = 0;
+      Changed = false;
+      if (LLVM_UNLIKELY(isBundlingEnabled())) {
+        MCFragment *Prev = nullptr;
+        for (MCFragment &F : Sec) {
+          F.Offset = Offset;
+          relaxFragment(F);
+          if (F.hasInstructions()) {
+            layoutBundle(Prev, &F);
+            Offset = F.Offset;
+          }
+          Prev = &F;
+          if (F.Offset != Offset) {
+            F.Offset = Offset;
+            Changed = true;
+          }
+          Offset += computeFragmentSize(F);
+        }
+      } else {
+        for (MCFragment &F : Sec) {
+          if (F.Offset != Offset) {
+            F.Offset = Offset;
+            Changed = true;
+          }
+          relaxFragment(F);
+          Offset += computeFragmentSize(F);
+        }
+      }
+
+      Changed |= OldSize != Offset;
+      ChangedAny |= Changed;
+      OldSize = Offset;
+    } while (Changed && --MaxIter);
+    if (MaxIter == 0)
+      return false;
+  }
+  return ChangedAny;
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)

llvm/lib/MC/MCSection.cpp

@@ -23,8 +23,8 @@ using namespace llvm;
 MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText,
                      bool IsVirtual, MCSymbol *Begin)
     : Begin(Begin), BundleGroupBeforeFirstInst(false), HasInstructions(false),
-      HasLayout(false), IsRegistered(false), IsText(IsText),
-      IsVirtual(IsVirtual), Name(Name), Variant(V) {
+      IsRegistered(false), IsText(IsText), IsVirtual(IsVirtual), Name(Name),
+      Variant(V) {
   DummyFragment.setParent(this);
   // The initial subsection number is 0. Create a fragment list.
   CurFragList = &Subsections.emplace_back(0u, FragList{}).second;