[MIPS] Optimize sortRelocs for o32

The o32 ABI specifies:

> Each relocation type of R_MIPS_HI16 must have an associated R_MIPS_LO16 entry immediately following it in the list of relocations. [...] the addend AHL is computed as (AHI << 16) + (short)ALO

In practice, the high-part and low-part relocations may not be adjacent
in assembly files, requiring the assembler to reorder relocations.
http://reviews.llvm.org/D19718 performed the reordering, but did not
optimize for the common case where a %lo immediately follows its
matching %hi. The quadratic time complexity could make sections with
many relocations very slow to process.

This patch implements the fast path, simplifies the code, and makes the
behavior more similar to GNU assembler (for the .rel.mips_hilo_8b test).
We also remove `OriginalSymbol`, removing overhead for other targets.

Fix #104562

Pull Request: #104723

Author: MaskRay

llvm/include/llvm/MC/MCELFObjectWriter.h

@@ -37,16 +37,14 @@ struct ELFRelocationEntry {
   const MCSymbolELF *Symbol; // The symbol to relocate with.
   unsigned Type;   // The type of the relocation.
   uint64_t Addend; // The addend to use.
-  const MCSymbolELF *OriginalSymbol; // The original value of Symbol if we changed it.
 
   ELFRelocationEntry(uint64_t Offset, const MCSymbolELF *Symbol, unsigned Type,
-                     uint64_t Addend, const MCSymbolELF *OriginalSymbol)
-      : Offset(Offset), Symbol(Symbol), Type(Type), Addend(Addend),
-        OriginalSymbol(OriginalSymbol) {}
+                     uint64_t Addend)
+      : Offset(Offset), Symbol(Symbol), Type(Type), Addend(Addend) {}
 
   void print(raw_ostream &Out) const {
     Out << "Off=" << Offset << ", Sym=" << Symbol << ", Type=" << Type
-        << ", Addend=" << Addend << ", OriginalSymbol=" << OriginalSymbol;
+        << ", Addend=" << Addend;
   }
 
   LLVM_DUMP_METHOD void dump() const { print(errs()); }

llvm/lib/MC/ELFObjectWriter.cpp

@@ -1453,7 +1453,7 @@ void ELFObjectWriter::recordRelocation(MCAssembler &Asm,
         SecA ? cast<MCSymbolELF>(SecA->getBeginSymbol()) : nullptr;
     if (SectionSymbol)
       SectionSymbol->setUsedInReloc();
-    ELFRelocationEntry Rec(FixupOffset, SectionSymbol, Type, Addend, SymA);
+    ELFRelocationEntry Rec(FixupOffset, SectionSymbol, Type, Addend);
     Relocations[&FixupSection].push_back(Rec);
     return;
   }
@@ -1468,7 +1468,7 @@ void ELFObjectWriter::recordRelocation(MCAssembler &Asm,
     else
       RenamedSymA->setUsedInReloc();
   }
-  ELFRelocationEntry Rec(FixupOffset, RenamedSymA, Type, Addend, SymA);
+  ELFRelocationEntry Rec(FixupOffset, RenamedSymA, Type, Addend);
   Relocations[&FixupSection].push_back(Rec);
 }
 

llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp

@@ -40,11 +40,6 @@ struct MipsRelocationEntry {
   bool Matched = false;       ///< Is this relocation part of a match.
 
   MipsRelocationEntry(const ELFRelocationEntry &R) : R(R) {}
-
-  void print(raw_ostream &Out) const {
-    R.print(Out);
-    Out << ", Matched=" << Matched;
-  }
 };
 
 class MipsELFObjectWriter : public MCELFObjectTargetWriter {
@@ -134,8 +129,7 @@ static unsigned getMatchingLoType(const ELFRelocationEntry &Reloc) {
   if (Type == ELF::R_MIPS16_HI16)
     return ELF::R_MIPS16_LO16;
 
-  if (Reloc.OriginalSymbol &&
-      Reloc.OriginalSymbol->getBinding() != ELF::STB_LOCAL)
+  if (Reloc.Symbol && Reloc.Symbol->getBinding() != ELF::STB_LOCAL)
     return ELF::R_MIPS_NONE;
 
   if (Type == ELF::R_MIPS_GOT16)
@@ -148,43 +142,11 @@ static unsigned getMatchingLoType(const ELFRelocationEntry &Reloc) {
   return ELF::R_MIPS_NONE;
 }
 
-/// Determine whether a relocation (X) matches the one given in R.
-///
-/// A relocation matches if:
-/// - It's type matches that of a corresponding low part. This is provided in
-///   MatchingType for efficiency.
-/// - It's based on the same symbol.
-/// - It's offset of greater or equal to that of the one given in R.
-///   It should be noted that this rule assumes the programmer does not use
-///   offsets that exceed the alignment of the symbol. The carry-bit will be
-///   incorrect if this is not true.
-///
-/// A matching relocation is unbeatable if:
-/// - It is not already involved in a match.
-/// - It's offset is exactly that of the one given in R.
-static FindBestPredicateResult isMatchingReloc(const MipsRelocationEntry &X,
-                                               const ELFRelocationEntry &R,
-                                               unsigned MatchingType) {
-  if (X.R.Type == MatchingType && X.R.OriginalSymbol == R.OriginalSymbol) {
-    if (!X.Matched && X.R.Addend == R.Addend)
-      return FindBest_PerfectMatch;
-    else if (X.R.Addend >= R.Addend)
-      return FindBest_Match;
-  }
-  return FindBest_NoMatch;
-}
-
-/// Determine whether Candidate or PreviousBest is the better match.
-/// The return value is true if Candidate is the better match.
-///
-/// A matching relocation is a better match if:
-/// - It has a smaller addend.
-/// - It is not already involved in a match.
-static bool compareMatchingRelocs(const MipsRelocationEntry &Candidate,
-                                  const MipsRelocationEntry &PreviousBest) {
-  if (Candidate.R.Addend != PreviousBest.R.Addend)
-    return Candidate.R.Addend < PreviousBest.R.Addend;
-  return PreviousBest.Matched && !Candidate.Matched;
+// Determine whether a relocation X is a low-part and matches the high-part R
+// perfectly by symbol and addend.
+static bool isMatchingReloc(unsigned MatchingType, const ELFRelocationEntry &R,
+                            const ELFRelocationEntry &X) {
+  return X.Type == MatchingType && X.Symbol == R.Symbol && X.Addend == R.Addend;
 }
 
 MipsELFObjectWriter::MipsELFObjectWriter(uint8_t OSABI,
@@ -413,58 +375,51 @@ void MipsELFObjectWriter::sortRelocs(const MCAssembler &Asm,
   if (hasRelocationAddend())
     return;
 
-  if (Relocs.size() < 2)
-    return;
-
   // Sort relocations by the address they are applied to.
   llvm::sort(Relocs,
              [](const ELFRelocationEntry &A, const ELFRelocationEntry &B) {
                return A.Offset < B.Offset;
              });
 
+  // Place relocations in a list for reorder convenience. Hi16 contains the
+  // iterators of high-part relocations.
   std::list<MipsRelocationEntry> Sorted;
-  std::list<ELFRelocationEntry> Remainder;
-
-  // Separate the movable relocations (AHL relocations using the high bits) from
-  // the immobile relocations (everything else). This does not preserve high/low
-  // matches that already existed in the input.
-  copy_if_else(Relocs.begin(), Relocs.end(), std::back_inserter(Remainder),
-               std::back_inserter(Sorted), [](const ELFRelocationEntry &Reloc) {
-                 return getMatchingLoType(Reloc) != ELF::R_MIPS_NONE;
-               });
+  SmallVector<std::list<MipsRelocationEntry>::iterator, 0> Hi16;
+  for (auto &R : Relocs) {
+    Sorted.push_back(R);
+    if (getMatchingLoType(R) != ELF::R_MIPS_NONE)
+      Hi16.push_back(std::prev(Sorted.end()));
+  }
 
-  for (auto &R : Remainder) {
+  for (auto I : Hi16) {
+    auto &R = I->R;
     unsigned MatchingType = getMatchingLoType(R);
-    assert(MatchingType != ELF::R_MIPS_NONE &&
-           "Wrong list for reloc that doesn't need a match");
-
-    // Find the best matching relocation for the current high part.
-    // See isMatchingReloc for a description of a matching relocation and
-    // compareMatchingRelocs for a description of what 'best' means.
-    auto InsertionPoint =
-        find_best(Sorted.begin(), Sorted.end(),
-                  [&R, &MatchingType](const MipsRelocationEntry &X) {
-                    return isMatchingReloc(X, R, MatchingType);
-                  },
-                  compareMatchingRelocs);
-
-    // If we matched then insert the high part in front of the match and mark
-    // both relocations as being involved in a match. We only mark the high
-    // part for cosmetic reasons in the debug output.
+    // If the next relocation is a perfect match, continue;
+    if (std::next(I) != Sorted.end() &&
+        isMatchingReloc(MatchingType, R, std::next(I)->R))
+      continue;
+    // Otherwise, find the best matching low-part relocation with the following
+    // criteria. It must have the same symbol and its addend is no lower than
+    // that of the current high-part.
     //
-    // If we failed to find a match then the high part is orphaned. This is not
-    // permitted since the relocation cannot be evaluated without knowing the
-    // carry-in. We can sometimes handle this using a matching low part that is
-    // already used in a match but we already cover that case in
-    // isMatchingReloc and compareMatchingRelocs. For the remaining cases we
-    // should insert the high part at the end of the list. This will cause the
-    // linker to fail but the alternative is to cause the linker to bind the
-    // high part to a semi-matching low part and silently calculate the wrong
-    // value. Unfortunately we have no means to warn the user that we did this
-    // so leave it up to the linker to complain about it.
-    if (InsertionPoint != Sorted.end())
-      InsertionPoint->Matched = true;
-    Sorted.insert(InsertionPoint, R)->Matched = true;
+    // (1) %lo with a smaller offset is preferred.
+    // (2) %lo with the same offset that is unmatched is preferred.
+    // (3) later %lo is preferred.
+    auto Best = Sorted.end();
+    for (auto J = Sorted.begin(); J != Sorted.end(); ++J) {
+      auto &R1 = J->R;
+      if (R1.Type == MatchingType && R.Symbol == R1.Symbol &&
+          R.Addend <= R1.Addend &&
+          (Best == Sorted.end() || R1.Addend < Best->R.Addend ||
+           (!Best->Matched && R1.Addend == Best->R.Addend)))
+        Best = J;
+    }
+    if (Best != Sorted.end() && R.Addend == Best->R.Addend)
+      Best->Matched = true;
+
+    // Move the high-part before the low-part, or if not found, the end of the
+    // list. The unmatched high-part will lead to a linker warning/error.
+    Sorted.splice(Best, Sorted, I);
   }
 
   assert(Relocs.size() == Sorted.size() && "Some relocs were not consumed");

llvm/test/MC/Mips/sort-relocation-table.s

@@ -150,8 +150,8 @@
 	lui $2, %hi(sym1)
 
 # CHECK-LABEL: Section ({{[0-9]+}}) .rel.mips_hilo_8b {
-# CHECK-NEXT:    0x8 R_MIPS_HI16 sym1
 # CHECK-NEXT:    0x0 R_MIPS_LO16 sym1
+# CHECK-NEXT:    0x8 R_MIPS_HI16 sym1
 # CHECK-NEXT:    0x4 R_MIPS_LO16 sym1
 # CHECK-NEXT:  }
 
@@ -331,8 +331,8 @@
 	lui $2, %got(local1)
 
 # CHECK-LABEL: Section ({{[0-9]+}}) .rel.mips_gotlo_8b {
-# CHECK-NEXT:    0x8 R_MIPS_GOT16 .text
 # CHECK-NEXT:    0x0 R_MIPS_LO16 .text
+# CHECK-NEXT:    0x8 R_MIPS_GOT16 .text
 # CHECK-NEXT:    0x4 R_MIPS_LO16 .text
 # CHECK-NEXT:  }
 
@@ -372,9 +372,9 @@
 # CHECK-LABEL: Section ({{[0-9]+}}) .rel.mips_gotlo_10 {
 # CHECK-NEXT:    0x0 R_MIPS_GOT16 .text
 # CHECK-NEXT:    0x4 R_MIPS_LO16 .text
+# CHECK-NEXT:    0x8 R_MIPS_GOT16 .text
 # CHECK-NEXT:    0xC R_MIPS_GOT16 .text
 # CHECK-NEXT:    0x10 R_MIPS_LO16 .text
-# CHECK-NEXT:    0x8 R_MIPS_GOT16 .text
 # CHECK-NEXT:  }
 
 # Finally, do test 2 for R_MIPS_GOT16 on external symbols to prove they are