[llvm] r360200 - Revert r360194 "[JITLink] Add support for MachO .alt_entry atoms."
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Tue May 7 15:19:29 PDT 2019
Author: lhames
Date: Tue May 7 15:19:29 2019
New Revision: 360200
URL: http://llvm.org/viewvc/llvm-project?rev=360200&view=rev
Log:
Revert r360194 "[JITLink] Add support for MachO .alt_entry atoms."
The testcase is asserting on some bots - reverting while I investigate.
Modified:
llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp
llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.h
llvm/trunk/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp
llvm/trunk/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s
Modified: llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp?rev=360200&r1=360199&r2=360200&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp Tue May 7 15:19:29 2019
@@ -44,33 +44,6 @@ void MachOAtomGraphBuilder::addCustomAto
CustomAtomizeFunctions[SectionName] = std::move(Atomizer);
}
-bool MachOAtomGraphBuilder::areLayoutLocked(const Atom &A, const Atom &B) {
- // If these atoms are the same then they're trivially "locked".
- if (&A == &B)
- return true;
-
- // If A and B are different, check whether either is undefined. (in which
- // case they are not locked).
- if (!A.isDefined() || !B.isDefined())
- return false;
-
- // A and B are different, but they're both defined atoms. We need to check
- // whether they're part of the same alt_entry chain.
- auto &DA = static_cast<const DefinedAtom &>(A);
- auto &DB = static_cast<const DefinedAtom &>(B);
-
- auto AStartItr = AltEntryStarts.find(&DA);
- if (AStartItr == AltEntryStarts.end()) // If A is not in a chain bail out.
- return false;
-
- auto BStartItr = AltEntryStarts.find(&DB);
- if (BStartItr == AltEntryStarts.end()) // If B is not in a chain bail out.
- return false;
-
- // A and B are layout locked if they're in the same chain.
- return AStartItr->second == BStartItr->second;
-}
-
unsigned
MachOAtomGraphBuilder::getPointerSize(const object::MachOObjectFile &Obj) {
return Obj.is64Bit() ? 8 : 4;
@@ -153,9 +126,6 @@ Error MachOAtomGraphBuilder::addNonCusto
DenseMap<MachOSection *, AddrToAtomMap> SecToAtoms;
DenseMap<MachOSection *, unsigned> FirstOrdinal;
- std::vector<DefinedAtom *> AltEntryAtoms;
-
- DenseSet<StringRef> ProcessedSymbols; // Used to check for duplicate defs.
for (auto SymI = Obj.symbol_begin(), SymE = Obj.symbol_end(); SymI != SymE;
++SymI) {
@@ -165,14 +135,6 @@ Error MachOAtomGraphBuilder::addNonCusto
if (!Name)
return Name.takeError();
- // Bail out on duplicate definitions: There should never be more than one
- // definition for a symbol in a given object file.
- if (ProcessedSymbols.count(*Name))
- return make_error<JITLinkError>("Duplicate definition within object: " +
- *Name);
- else
- ProcessedSymbols.insert(*Name);
-
auto Addr = Sym.getAddress();
if (!Addr)
return Addr.takeError();
@@ -227,35 +189,24 @@ Error MachOAtomGraphBuilder::addNonCusto
auto &Sec = SecByIndexItr->second;
- auto &DA = G->addDefinedAtom(Sec.getGenericSection(), *Name, *Addr,
- std::max(Sym.getAlignment(), 1U));
+ auto &A = G->addDefinedAtom(Sec.getGenericSection(), *Name, *Addr,
+ std::max(Sym.getAlignment(), 1U));
- DA.setGlobal(Flags & object::SymbolRef::SF_Global);
- DA.setExported(Flags & object::SymbolRef::SF_Exported);
- DA.setWeak(Flags & object::SymbolRef::SF_Weak);
-
- DA.setCallable(*SymType & object::SymbolRef::ST_Function);
-
- // Check alt-entry.
- {
- uint16_t NDesc = 0;
- if (Obj.is64Bit())
- NDesc = Obj.getSymbolTableEntry(SymI->getRawDataRefImpl()).n_desc;
- else
- NDesc = Obj.getSymbolTableEntry(SymI->getRawDataRefImpl()).n_desc;
- if (NDesc & MachO::N_ALT_ENTRY)
- AltEntryAtoms.push_back(&DA);
- }
+ A.setGlobal(Flags & object::SymbolRef::SF_Global);
+ A.setExported(Flags & object::SymbolRef::SF_Exported);
+ A.setWeak(Flags & object::SymbolRef::SF_Weak);
+
+ A.setCallable(*SymType & object::SymbolRef::ST_Function);
LLVM_DEBUG({
dbgs() << " Added " << *Name
<< " addr: " << format("0x%016" PRIx64, *Addr)
- << ", align: " << DA.getAlignment()
+ << ", align: " << A.getAlignment()
<< ", section: " << Sec.getGenericSection().getName() << "\n";
});
auto &SecAtoms = SecToAtoms[&Sec];
- SecAtoms[DA.getAddress() - Sec.getAddress()] = &DA;
+ SecAtoms[A.getAddress() - Sec.getAddress()] = &A;
}
// Add anonymous atoms.
@@ -312,50 +263,6 @@ Error MachOAtomGraphBuilder::addNonCusto
}
}
- LLVM_DEBUG(dbgs() << "Adding alt-entry starts\n");
-
- // Sort alt-entry atoms by address in ascending order.
- llvm::sort(AltEntryAtoms.begin(), AltEntryAtoms.end(),
- [](const DefinedAtom *LHS, const DefinedAtom *RHS) {
- return LHS->getAddress() < RHS->getAddress();
- });
-
- // Process alt-entry atoms in address order to build the table of alt-entry
- // atoms to alt-entry chain starts.
- for (auto *DA : AltEntryAtoms) {
- assert(!AltEntryStarts.count(DA) && "Duplicate entry in AltEntryStarts");
-
- // DA is an alt-entry atom. Look for the predecessor atom that it is locked
- // to, bailing out if we do not find one.
- auto AltEntryPred = G->findAtomByAddress(DA->getAddress() - 1);
- if (!AltEntryPred)
- return AltEntryPred.takeError();
-
- // Add a LayoutNext edge from the predecessor to this atom.
- AltEntryPred->addEdge(Edge::LayoutNext, 0, *DA, 0);
-
- // Check to see whether the predecessor itself is an alt-entry atom.
- auto AltEntryStartItr = AltEntryStarts.find(&*AltEntryPred);
- if (AltEntryStartItr != AltEntryStarts.end()) {
- // If the predecessor was an alt-entry atom then re-use its value.
- AltEntryStarts[DA] = AltEntryStartItr->second;
- LLVM_DEBUG({
- dbgs() << " " << *DA << " -> " << *AltEntryStartItr->second
- << " (based on existing entry for " << *AltEntryPred << ")\n";
- });
- } else {
- // If the predecessor does not have an entry then add an entry for this
- // atom (i.e. the alt_entry atom) and a self-reference entry for the
- /// predecessory atom that is the start of this chain.
- AltEntryStarts[&*AltEntryPred] = &*AltEntryPred;
- AltEntryStarts[DA] = &*AltEntryPred;
- LLVM_DEBUG({
- dbgs() << " " << *AltEntryPred << " -> " << *AltEntryPred << "\n"
- << " " << *DA << " -> " << *AltEntryPred << "\n";
- });
- }
- }
-
return Error::success();
}
Modified: llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.h?rev=360200&r1=360199&r2=360200&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.h (original)
+++ llvm/trunk/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.h Tue May 7 15:19:29 2019
@@ -96,10 +96,6 @@ protected:
virtual Error addRelocations() = 0;
- /// Returns true if Atom A and Atom B are at a fixed offset from one another
- /// (i.e. if they're part of the same alt-entry chain).
- bool areLayoutLocked(const Atom &A, const Atom &B);
-
private:
static unsigned getPointerSize(const object::MachOObjectFile &Obj);
static support::endianness getEndianness(const object::MachOObjectFile &Obj);
@@ -112,7 +108,6 @@ private:
const object::MachOObjectFile &Obj;
std::unique_ptr<AtomGraph> G;
- DenseMap<const DefinedAtom *, const DefinedAtom *> AltEntryStarts;
DenseMap<unsigned, MachOSection> Sections;
StringMap<CustomAtomizeFunction> CustomAtomizeFunctions;
Optional<MachOSection> CommonSymbolsSection;
Modified: llvm/trunk/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp?rev=360200&r1=360199&r2=360200&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp Tue May 7 15:19:29 2019
@@ -181,20 +181,19 @@ private:
MachOX86RelocationKind DeltaKind;
Atom *TargetAtom;
uint64_t Addend;
- if (areLayoutLocked(AtomToFix, *FromAtom)) {
+ if (&AtomToFix == &*FromAtom) {
TargetAtom = ToAtom;
DeltaKind = (SubRI.r_length == 3) ? Delta64 : Delta32;
Addend = FixupValue + (FixupAddress - FromAtom->getAddress());
// FIXME: handle extern 'from'.
- } else if (areLayoutLocked(AtomToFix, *ToAtom)) {
+ } else if (&AtomToFix == ToAtom) {
TargetAtom = &*FromAtom;
DeltaKind = (SubRI.r_length == 3) ? NegDelta64 : NegDelta32;
Addend = FixupValue - (FixupAddress - ToAtom->getAddress());
} else {
// AtomToFix was neither FromAtom nor ToAtom.
return make_error<JITLinkError>("SUBTRACTOR relocation must fix up "
- "either 'A' or 'B' (or an atom in one "
- "of their alt-entry groups)");
+ "either 'A' or 'B'");
}
return PairRelocInfo(DeltaKind, TargetAtom, Addend);
Modified: llvm/trunk/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s?rev=360200&r1=360199&r2=360200&view=diff
==============================================================================
--- llvm/trunk/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s (original)
+++ llvm/trunk/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s Tue May 7 15:19:29 2019
@@ -129,19 +129,13 @@ Lanon_minuend_quad:
Lanon_minuend_long:
.long Lanon_minuend_long - named_data + 2
+
# Named quad storage target (first named atom in __data).
.globl named_data
.p2align 3
named_data:
.quad 0x2222222222222222
-# An alt-entry point for named_data
- .globl named_data_alt_entry
- .p2align 3
- .alt_entry named_data_alt_entry
-named_data_alt_entry:
- .quad 0
-
# Check X86_64_RELOC_UNSIGNED / extern handling by putting the address of a
# local named function in a pointer variable.
#
@@ -207,60 +201,4 @@ minuend_quad3:
minuend_long3:
.long minuend_long3 - named_data + 2
-# Check X86_64_RELOC_SUBTRACTOR handling for exprs of the form
-# "A: .quad/long B - C + D", where 'B' or 'C' is at a fixed offset from 'A'
-# (i.e. is part of an alt_entry chain that includes 'A').
-#
-# Check "A: .long B - C + D" where 'B' is an alt_entry for 'A'.
-# jitlink-check: *{4}subtractor_with_alt_entry_minuend_long = (subtractor_with_alt_entry_minuend_long_B - named_data + 2)[31:0]
- .globl subtractor_with_alt_entry_minuend_long
- .p2align 2
-subtractor_with_alt_entry_minuend_long:
- .long subtractor_with_alt_entry_minuend_long_B - named_data + 2
-
- .globl subtractor_with_alt_entry_minuend_long_B
- .p2align 2
- .alt_entry subtractor_with_alt_entry_minuend_long_B
-subtractor_with_alt_entry_minuend_long_B:
- .long 0
-
-# Check "A: .quad B - C + D" where 'B' is an alt_entry for 'A'.
-# jitlink-check: *{8}subtractor_with_alt_entry_minuend_quad = (subtractor_with_alt_entry_minuend_quad_B - named_data + 2)
- .globl subtractor_with_alt_entry_minuend_quad
- .p2align 3
-subtractor_with_alt_entry_minuend_quad:
- .quad subtractor_with_alt_entry_minuend_quad_B - named_data + 2
-
- .globl subtractor_with_alt_entry_minuend_quad_B
- .p2align 3
- .alt_entry subtractor_with_alt_entry_minuend_quad_B
-subtractor_with_alt_entry_minuend_quad_B:
- .quad 0
-
-# Check "A: .long B - C + D" where 'C' is an alt_entry for 'A'.
-# jitlink-check: *{4}subtractor_with_alt_entry_subtrahend_long = (named_data - subtractor_with_alt_entry_subtrahend_long_B + 2)[31:0]
- .globl subtractor_with_alt_entry_subtrahend_long
- .p2align 2
-subtractor_with_alt_entry_subtrahend_long:
- .long named_data - subtractor_with_alt_entry_subtrahend_long_B + 2
-
- .globl subtractor_with_alt_entry_subtrahend_long_B
- .p2align 2
- .alt_entry subtractor_with_alt_entry_subtrahend_long_B
-subtractor_with_alt_entry_subtrahend_long_B:
- .long 0
-
-# Check "A: .quad B - C + D" where 'B' is an alt_entry for 'A'.
-# jitlink-check: *{8}subtractor_with_alt_entry_subtrahend_quad = (named_data - subtractor_with_alt_entry_subtrahend_quad_B + 2)
- .globl subtractor_with_alt_entry_subtrahend_quad
- .p2align 3
-subtractor_with_alt_entry_subtrahend_quad:
- .quad named_data - subtractor_with_alt_entry_subtrahend_quad_B + 2
-
- .globl subtractor_with_alt_entry_subtrahend_quad_B
- .p2align 3
- .alt_entry subtractor_with_alt_entry_subtrahend_quad_B
-subtractor_with_alt_entry_subtrahend_quad_B:
- .quad 0
-
.subsections_via_symbols
More information about the llvm-commits
mailing list