[llvm] [MIPS] Optimize sortRelocs for o32 (PR #104723)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 22 23:59:18 PDT 2024
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/104723
>From 665bd5133cfac33d8610233ee5c96c87634220d9 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sun, 18 Aug 2024 16:15:47 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
=?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.5-bogner
---
llvm/include/llvm/MC/MCELFObjectWriter.h | 8 +-
llvm/lib/MC/ELFObjectWriter.cpp | 4 +-
.../Mips/MCTargetDesc/MipsELFObjectWriter.cpp | 127 ++++++------------
llvm/test/MC/Mips/sort-relocation-table.s | 6 +-
4 files changed, 49 insertions(+), 96 deletions(-)
diff --git a/llvm/include/llvm/MC/MCELFObjectWriter.h b/llvm/include/llvm/MC/MCELFObjectWriter.h
index d17fc931d1561c..b09e3bbffad329 100644
--- a/llvm/include/llvm/MC/MCELFObjectWriter.h
+++ b/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()); }
diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 35d0a2eb52dfc7..62a4b5347f8299 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/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);
}
diff --git a/llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp b/llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp
index 19b6bf0da22a60..9cb1368719c01d 100644
--- a/llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp
+++ b/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 ( same symbol/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");
diff --git a/llvm/test/MC/Mips/sort-relocation-table.s b/llvm/test/MC/Mips/sort-relocation-table.s
index cc951956fd24a0..7d126ba9f049d8 100644
--- a/llvm/test/MC/Mips/sort-relocation-table.s
+++ b/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
More information about the llvm-commits
mailing list