This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [wwwdocs] Add page about CFG branch
On Tue, Nov 20, 2001 at 09:00:01AM +0100, Andreas Jaeger wrote:
>
> Gerald, is this version ok to commit?
This still has some grammar problems, some of them bad enough that I
can't figure out what the sentence is supposed to mean. Also, there
are a number of undefined terms.
> +<title>Improving GCC's Infrastructure (Central-Flow-Graph)</title>
Here and elsewhere - *control* flow graph, surely? Also, in English
both the capitalization and the dashes look funny: suggest
Improving GCC's infrastructure (Control flow graph)
> +The work is done on a branch in GCC's CVS
> +tree called <code>cfg-branch</code>, which is available to
> +experiment.
I believe you want
... available to experiment in.
here, but there are a few other words that might be more appropriate.
> As the patches stabilize, they can be submitted for review
> +into the mainline development tree.
One does not "review into" anything. "review and acceptance into" is
better, but still awkward.
[A technical aside; I'm not comfortable with the idea of a branch with
lots of experimental patches in it, which you then try to pull out and
merge into mainline one at a time - I think you'll drown in ordering
problems and merge conflicts.]
> Please contact <a
> +href="mailto:jh@suse.cz";>Jan Hubicka <jh@suse.cz></a> if you
> +like to contribute.</p>
... if you would like to contribute.
> +<p>GCC is an aged project and needs a bit of modernization. Some new
> +infrastructure has been implemented in the recent years, but lots of
> +existing optimizers do not use it. The main mission of work on the
> +CFG branch is to avoid such archaism and develop modern infrastructure
> +to make developing of new optimizers easier.</p>
This whole paragraph reads awkwardly, although there isn't any single
problem I can point to other than nits. I can only suggest a total
rewrite:
The purpose of work on the CFG branch is to update GCC's older
optimization passes to use the modern infrastructure which was
not available when they were written. A second purpose is to
continue improving the infrastructure for RTL-based optimizations.
> +<p>The work concentrates currently on the Central Flow Graph (aka cfg)
> +datastructure, originaly contributed by Richard Henderson. We would
> +like to convert the whole compiler to use it and keep it accurate
> +during the optimization passes. This allows to attach information
> +directly to the structure, instead of using notes in the instruction
> +stream that are difficult to maintain up-to-date during
> +optimizing.</p>
Again, a total rewrite:
Currently, work concentrates on the control flow graph (CFG)
data structure, originally contributed by Richard Henderson.
We would like to convert all of the RTL-based optimizer passes
to use it and keep it accurate. This will allow us to store
more information directly in this structure, instead of using
"notes" in the instruction stream which are difficult to keep
accurate during optimization.
I'd like here to make some general points about English grammar. "This
allows to <action>" is ill-formed; you have to say "This allows
<actor> to <action>". When you want <actor> to be left unspecified,
use "us" if the speaker should be part of the unspecified category,
"them" otherwise.
"This allows us to throw out ten megabytes of obsolete code."
"This allows them to ignore all zoning legislation."
"The" is not applied to "work". A good rule of thumb for when you
need "a" or "the" in front of a noun: if the noun describes an object
which is _not unique_ and _definite_, you use them. If the object is
either _unique_ or _indefinite_, you don't.
the water (in this lake) water (in general)
the compiler GCC
a project work
"Work concentrates currently" is not wrong, but most native speakers
would write "currently concentrates".
> +<p>In addition to providing the infrastructure and updating the
> +existing compiler, new optimization passes are implemented.
Here's another nice example of "the" problems. "Providing the
infrastructure" is wrong, but either "improving the infrastructure" or
"providing infrastructure" would be fine. The choice of verb
determines whether "infrastructure" is definite or indefinite, thus
whether it needs a leading "the".
Me, I would say
...improving the infrastructure for RTL optimizers and
updating the existing RTL-based optimization passes...
It never hurts to be specific.
You can't switch to past tense and passive voice in the middle of a
sentence. "New optimization passes are implemented" clashes with "In
addition to <doing stuff>". The best alternative is
... we are implementing new optimization passes.
> We plan to
> +implement optimization that deal with code duplication first -
optimizations that perform code duplication first, such as
> +superblock formation, loop peeling and re-implementing the loop
> +unrolling pass.</p>
superblock formation and loop peeling. We also plan to reimplement
the loop unrolling pass.
Here's the first place where you use undefined terms - loop unrolling
is a well known concept, but superblock formation and loop peeling
aren't. It would be helpful if you at least provided pointers to
articles on these optimizations.
> +<p>The branch was created from the development mainline on 12 November
> +2001 and its CVS tag is called 'cfg-branch'.</p>
Run-on. "...on 12 November 2001. Its CVS tag..."
Suggest <code>cfg-branch</code>.
Don't use single quotes for anything other than apostrophes, except in
code examples; you cannot predict whether the viewer sees a vertical
line, a true apostrophe, or a right single quote.
> +<dd>Before the branching the CFG routines have been reorganized,
> +cleaned up and made easier to use. Each instruction now keeps
> +information about the basic block so updating of the basic block
> +boundaries can be done transparently.</dd>
Active voice would be better here:
Before branching, we reorganized and cleaned up the CFG
routines to make them easier to use.
The second sentence does not hold together very well. _Which_ basic
block does it keep information about? (The one it's in, in fact, but
such is not obvious from the sentence.) And the second half does not
obviously follow from the first, unless you already know what basic
block notes are and why they're trouble.
Each RTL instruction now contains a pointer to its basic
block. It is no longer necessary to adjust basic block
boundary notes when instructions are reordered.
[assuming that that is in fact true]
> +<li>New cfglayout.c based on bb-reorder pass allowing code
> +duplication</li>
New <code>cfglayout.c</code> based on the basic block
reordering pass, and capable of code duplication.
What's a cfglayout.c?
Might want to insert an explanation of why this is a useful
optimization - "code duplication" with no further context is generally
considered a bad thing.
All these entries should be complete sentences ending with periods.
> +<li>Fix to loop discovery code (by Josef Zlomek)</li>
A fix to the loop discovery code, by Josef Zlomek.
Say what the bug was.
> +<dd>Use tail duplication to form superblocks for easy optimization
Uses
> +by other passes. The code is already present on the branch and give
and gives
> +<dd>Tracer peels loop with one predicted iteration. We should try
> +to peel more.</dd>
The tracer peels loops ...
"Tracer peels" is acceptable only if Tracer is someone's name.
"loops" needs to be plural, since "peels" implies that there is more
than one such loop.
Much more importantly than these grammar issues, I do not know what
these sentences mean. I've never heard of loop peeling, or the
tracer; it would help if these terms were defined.
> +<dd>Assigns each web different pseudo.
This sentence is missing a critical word between "web" and
"different", but I don't know which word you want, because I don't
know what a web is.
I think I've heard the term used in the context of the new register
allocator, though - what is the relationship between this pass and
that project?
> +This helps a number of passes (CSE, register allocator)
... passes, such as CSE and the register allocator.
> +and needs to be used as cleanup after
> +superblock formation.
It is also helpful in cleaning up code after superblock formation.
or
It must also be used to clean up after superblock formation.
depending on how disastrous it is if you don't do this.
What's a superblock?
> +The pass needs more work on updating debug information.</dd>
The pass needs to update debug information more carefully.
or
We need to work on updating debug information in this pass.
> +<dt>Avoid CFG rebuilds in the file toplev.c</dt>
Avoid rebuilding the CFG from <code>toplev.c</code>
or just
Avoid rebuilding the CFG from scratch
> +<dt>Highlevel branch prediction</dt>
High level
> +<p>Checkout the <code>cfg-branch</code> branch</p>
Check out the <code>cfg-branch</code> with the command
> +<p>Please post patches in the usual way to the gcc-patches list,
> +marked <code>[cfg-branch]</code>. As this is a branch, and not the
> +mainline, the usual maintainer rules do not apply. This branch is
Strike "and not the mainline".
> +Approval from the usual maintainers will be
> +needed when submitting patches from the branch onto the mainline.</p>
... from the branch for consideration for the mainline.
zw