[PATCH] Do not use layout-before to layout atoms.
Rui Ueyama
ruiu at google.com
Tue Mar 25 16:49:36 PDT 2014
Ping. Any comments?
On Mon, Mar 24, 2014 at 3:29 PM, Rui Ueyama <ruiu at google.com> wrote:
> Hi Bigcheese, shankarke, kledzik, atanasyan,
>
> 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.
>
> http://llvm-reviews.chandlerc.com/D3164
>
> Files:
> include/lld/Passes/LayoutPass.h
> lib/Passes/LayoutPass.cpp
> test/core/layout-error-test.objtxt
> test/core/layoutbefore-test.objtxt
>
> Index: include/lld/Passes/LayoutPass.h
> ===================================================================
> --- include/lld/Passes/LayoutPass.h
> +++ include/lld/Passes/LayoutPass.h
> @@ -55,10 +55,6 @@
> // 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);
>
> Index: lib/Passes/LayoutPass.cpp
> ===================================================================
> --- lib/Passes/LayoutPass.cpp
> +++ lib/Passes/LayoutPass.cpp
> @@ -159,8 +159,7 @@
>
> /// 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 @@
> }
> }
>
> -/// 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<DefinedAtom>(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 @@
> // 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));
>
> Index: test/core/layout-error-test.objtxt
> ===================================================================
> --- 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
> Index: test/core/layoutbefore-test.objtxt
> ===================================================================
> --- 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140325/cf0aa3c1/attachment.html>
More information about the llvm-commits
mailing list