From 8be4ce254a49cbf36253b7566a6fd4cb1c947a3e Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Thu, 27 Mar 2014 22:08:26 +0000 Subject: [PATCH] Do not use layout-before to layout atoms. Currently we use both layout-after and layout-before edges to specify atom orders in the resulting executable. We have a complex piece of code in LayoutPass.cpp to deal with both types of layout specifiers. (In the following description, I denote "Atom A having a layout-after edge to B" as "A -> B", and A's layout-before to B as "A => B".) However, that complexity is not really needed for this reason: If there are atoms such that A => B, B -> A is always satisifed, so using only layout- after relationships will yield the same result as the current code. Actually we have a piece of complex code that verifies that, for each A -> B, B => [ X => Y => ... => Z => ] A is satsified, where X, Y, ... Z are all zero-size atoms. We can get rid of the code from our codebase because layout- before is basically redundant. I think we can simplify the code for layout-after even more than this, but I want to just remove this pass for now for simplicity. Layout-before edges are still there for dead-stripping, so this change won't break it. We will remove layout-before in a followup patch once we fix the dead-stripping pass. Differential Revision: http://llvm-reviews.chandlerc.com/D3164 llvm-svn: 204966 --- lld/include/lld/Passes/LayoutPass.h | 4 -- lld/lib/Passes/LayoutPass.cpp | 75 +------------------------- lld/test/core/layout-error-test.objtxt | 22 -------- lld/test/core/layoutbefore-test.objtxt | 25 --------- 4 files changed, 1 insertion(+), 125 deletions(-) delete mode 100644 lld/test/core/layout-error-test.objtxt delete mode 100644 lld/test/core/layoutbefore-test.objtxt diff --git a/lld/include/lld/Passes/LayoutPass.h b/lld/include/lld/Passes/LayoutPass.h index 6ec213c6fd5d..3392480b1998 100644 --- a/lld/include/lld/Passes/LayoutPass.h +++ b/lld/include/lld/Passes/LayoutPass.h @@ -55,10 +55,6 @@ private: // reference type void buildInGroupTable(MutableFile::DefinedAtomRange &range); - // Build the PrecededBy Table as specified by the kindLayoutBefore - // reference type - void buildPrecededByTable(MutableFile::DefinedAtomRange &range); - // Build a map of Atoms to ordinals for sorting the atoms void buildOrdinalOverrideMap(MutableFile::DefinedAtomRange &range); diff --git a/lld/lib/Passes/LayoutPass.cpp b/lld/lib/Passes/LayoutPass.cpp index 37635cc26f21..6f1d4805579a 100644 --- a/lld/lib/Passes/LayoutPass.cpp +++ b/lld/lib/Passes/LayoutPass.cpp @@ -159,8 +159,7 @@ void LayoutPass::checkFollowonChain(MutableFile::DefinedAtomRange &range) { /// The function compares atoms by sorting atoms in the following order /// a) Sorts atoms by Section position preference -/// b) Sorts atoms by their ordinal overrides -/// (layout-after/layout-before/ingroup) +/// b) Sorts atoms by their ordinal overrides (layout-after/ingroup) /// c) Sorts atoms by their permissions /// d) Sorts atoms by their content /// e) Sorts atoms on how they appear using File Ordinality @@ -448,75 +447,6 @@ void LayoutPass::buildInGroupTable(MutableFile::DefinedAtomRange &range) { } } -/// This pass builds the followon tables using Preceded By relationships -/// The algorithm follows a very simple approach -/// a) If the targetAtom is not part of any root and the current atom is not -/// part of any root, create a chain with the current atom as root and -/// the targetAtom as following the current atom -/// b) Chain the targetAtom to the current Atom if the targetAtom is not part -/// of any chain and the currentAtom has no followOn's -/// c) If the targetAtom is part of a different tree and the root of the -/// targetAtom is itself, and if the current atom is not part of any root -/// chain all the atoms together -/// d) If the current atom has no followon and the root of the targetAtom is -/// not equal to the root of the current atom(the targetAtom is not in the -/// same chain), chain all the atoms that are lead by the targetAtom into -/// the current chain -void LayoutPass::buildPrecededByTable(MutableFile::DefinedAtomRange &range) { - ScopedTask task(getDefaultDomain(), "LayoutPass::buildPrecededByTable"); - // This table would convert precededby references to follow on - // references so that we have only one table - for (const DefinedAtom *ai : range) { - for (const Reference *r : *ai) { - if (r->kindNamespace() != lld::Reference::KindNamespace::all || - r->kindValue() != lld::Reference::kindLayoutBefore) - continue; - const DefinedAtom *targetAtom = dyn_cast(r->target()); - // Is the targetAtom not chained - if (_followOnRoots.count(targetAtom) == 0) { - // Is the current atom not part of any root? - if (_followOnRoots.count(ai) == 0) { - _followOnRoots[ai] = ai; - _followOnNexts[ai] = targetAtom; - _followOnRoots[targetAtom] = _followOnRoots[ai]; - continue; - } - if (_followOnNexts.count(ai) == 0) { - // Chain the targetAtom to the current Atom - // if the currentAtom has no followon references - _followOnNexts[ai] = targetAtom; - _followOnRoots[targetAtom] = _followOnRoots[ai]; - } - continue; - } - if (_followOnRoots.find(targetAtom)->second != targetAtom) - continue; - // Is the targetAtom in chain with the targetAtom as the root? - bool changeRoots = false; - if (_followOnRoots.count(ai) == 0) { - _followOnRoots[ai] = ai; - _followOnNexts[ai] = targetAtom; - _followOnRoots[targetAtom] = _followOnRoots[ai]; - changeRoots = true; - } else if (_followOnNexts.count(ai) == 0) { - // Chain the targetAtom to the current Atom - // if the currentAtom has no followon references - if (_followOnRoots[ai] != _followOnRoots[targetAtom]) { - _followOnNexts[ai] = targetAtom; - _followOnRoots[targetAtom] = _followOnRoots[ai]; - changeRoots = true; - } - } - // Change the roots of the targetAtom and its chain to - // the current atoms root - if (changeRoots) { - setChainRoot(_followOnRoots[targetAtom], _followOnRoots[ai]); - } - } - } -} - - /// Build an ordinal override map by traversing the followon chain, and /// assigning ordinals to each atom, if the atoms have their ordinals /// already assigned skip the atom and move to the next. This is the @@ -572,9 +502,6 @@ void LayoutPass::perform(std::unique_ptr &mergedFile) { // Build Ingroup reference table buildInGroupTable(atomRange); - // Build preceded by tables - buildPrecededByTable(atomRange); - // Check the structure of followon graph if running in debug mode. DEBUG(checkFollowonChain(atomRange)); diff --git a/lld/test/core/layout-error-test.objtxt b/lld/test/core/layout-error-test.objtxt deleted file mode 100644 index c9594d28f98c..000000000000 --- a/lld/test/core/layout-error-test.objtxt +++ /dev/null @@ -1,22 +0,0 @@ -# REQUIRES: asserts -# RUN: not lld -core --add-pass layout -mllvm -debug-only=LayoutPass \ -# RUN: %s 2> %t.err -# RUN: FileCheck %s -check-prefix=CHECK < %t.err - ---- -defined-atoms: - - name: fn - scope: global - references: - - kind: layout-before - offset: 0 - target: fn - - kind: in-group - offset: 0 - target: fn -... - -# CHECK: There's a cycle in a follow-on chain! -# CHECK: fn -# CHECK: layout-before: fn -# CHECK: in-group: fn diff --git a/lld/test/core/layoutbefore-test.objtxt b/lld/test/core/layoutbefore-test.objtxt deleted file mode 100644 index 038c14dc0fd4..000000000000 --- a/lld/test/core/layoutbefore-test.objtxt +++ /dev/null @@ -1,25 +0,0 @@ -# RUN: lld -core --add-pass layout %s | FileCheck %s -check-prefix=CHKORDER - ---- -defined-atoms: - - name: fn - scope: global - - - name: fn1 - scope: global - references: - - kind: layout-before - offset: 0 - target: fn - - name: fn2 - scope: global - references: - - kind: layout-before - offset: 0 - target: fn1 -... - - -# CHKORDER: - name: fn2 -# CHKORDER: - name: fn1 -# CHKORDER: - name: fn