[PATCH] Do not use layout-before to layout atoms.
Rui Ueyama
ruiu at google.com
Mon Mar 24 15:29:04 PDT 2014
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 --------------
A non-text attachment was scrubbed...
Name: D3164.1.patch
Type: text/x-patch
Size: 6420 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140324/54ee15fa/attachment.bin>
More information about the llvm-commits
mailing list