From: Mark Mitchell <mark@codesourcery.com>
To: Richard Kenner <kenner@vlsi1.ultra.nyu.edu>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: Patch to allow Ada to work with tree-ssa
Date: Tue, 22 Jun 2004 17:46:00 -0000 [thread overview]
Message-ID: <40D857AB.1020800@codesourcery.com> (raw)
In-Reply-To: <10406221307.AA05385@vlsi1.ultra.nyu.edu>
Richard Kenner wrote:
> In particular, you've just added an extra entry to ARRAY_REF and
> COMPONENT_REF, making these two common tree nodes bigger.
> (COMPONENT_REF, in particular, is used extensively in C++; every access
> to a class member variable is a COMPONENT_REF.) These extra entries are
> unnecessary in C and C++, meaning that for GCC's two most-used languages
> you've just increased memory usage with no benefit. If you need
> some additional stuff for Ada, please find a way that doesn't penalize
> C and C++.
>
>I can show you a GNU C program that needs them as well. We support
>variable-sized types.
>
>
I'm not disputing that you fixed a bug with these patches; what I'm
saying is that there must be a better way to fix it.
>This issue was discussed extensively on the GCC list and nobody had a
>better idea as to how to do it. Moreover, I sent email yesterday morning
>saying what my plans were and nobody objected.
>
>
Many of us cannot read GCC mail on a continuous basis. In any case, the
fact that you provided notice doesn't alter the fact that you've
increased the size of these nodes.
> Furthermore, you didn't update the c-tree.texi documentation.
>
>That's a C-specific file: these are language-independent nodes.
>I did document them in tree.def.
>
>
It's was originally a C and C++-specific file; now, it's the closest
thing we have to tree documentation. It certainly documents ARRAY_REF
and COMPONENT_REF.
> for example, the third operand in a COMPONENT_REF is defined in terms
> of the DECL_FIELD_OFFSET of the second operand; if that's true, why do
> we need to have it there explicitly? Presumably, this has something
> to do with variable-sized types, but the documentation does not say
> what that is.
>
>I'm not sure what you want and where. The nodes are defined even for
>fixed records.
>
>
In their plain English meaning, the words you wrote in tree.def make
these fields redundant; you have defined the values of these fields in
terms of information already available in the tree. You've suggested
that this is not actually true; that they are conveying some information
not already present -- but the words you've written do not make clear
what that is. For example, "Operand 2 is a copy of TYPE_MIN_VALUE of
the index." OK, then, why don't we just do TYPE_MIN_VALUE (TREE_TYE
(TREE_OEPRAND (x, 1)))? Why store this in the node? That is not
explained in your documentation.
>As to getting the data from DECL_FIELD_OFFSET (or TYPE_MIN_VALUE), that was
>my original plan since I also didn't like the idea of of increasing the size
>of the node. But *lots* of people were very much against that idea,
>including Diego, RTH, Jeff, and some I can't remember now. I still was
>hoping for an approach like that, but then I realized that PLACEHOLDER_EXPR
>makes it *not* redundant in any case, so I did what others wanted me to
>do, which is perhaps this approach.
>
>
I am very much against increasing the size of COMPONENT_REF and
ARRAY_REF for C++. I've suggested a technical solution: add a flag to
the nodes that says whether they have all the fields, or just the old
ones. Presumably, it will be very rare in C/C++ to have all the
fields. True, we don't have that yet -- but, I'm confident you can find
a way.
>Let's make the very pessimistic assumption that COMPONENT_REF are half of the
>remaining nodes and ARRAY_REF are half of what's left. So 25% of expr nodes
>grow 8% and 12% grow 17%. That's an increase of 4% in memory for expr nodes.
>Since they are 19% of all nodes, that's an overall memory increase of under
>1%. I think we can live with that.
>
>
No, we can't. It is precisely the 1% increases that have gotten us to
the bloated state we are in now. A patch a week with a 1% increase
means that we get close to doubling the memory usage every year. I just
spent a few days eliminating roughly 1% of the memory used by C++, and
I'm going to be aggressive aboiut preventing memory usage from increasing.
I recognize that you estimated conservatively, but we should not be
making changes that increase memory usage, especially not for permanent,
uncollectable nodes which these will be; in C++, most expressions live
for a long time, due to inlining and templates.
>If there is consensus after discussion that this patch is bad, I will, of
>course, revert it, but I'd *much* rather use it as a base and make
>improvements to it as needed, so progress can go forward, not backward.
>
>
That's fine with me if you will agree to implement the optimization
where these nodes are smaller when the extra fields are not needed, or
implement new tree nodes for VARIABLE_ARRAY_REF or
VARIABLE_COMPONENT_REF. An even easier change would be to embed the
additional fields in one of the existing operands; have the second
operand be a TREE_LIST, or some such, in the case where there is a need
for this additional information. Ugly, but effective.
--
Mark Mitchell
CodeSourcery, LLC
(916) 791-8304
mark@codesourcery.com
next prev parent reply other threads:[~2004-06-22 16:00 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-06-22 16:33 Richard Kenner
2004-06-22 17:46 ` Mark Mitchell [this message]
2004-06-22 18:17 ` Joseph S. Myers
2004-06-22 18:52 ` Richard Henderson
2004-06-22 19:37 ` Zack Weinberg
-- strict thread matches above, loose matches on Subject: below --
2004-07-29 19:02 Richard Kenner
2004-07-29 18:43 Richard Kenner
2004-07-29 17:12 Richard Kenner
2004-07-29 17:13 ` Diego Novillo
2004-07-29 17:57 ` Richard Henderson
2004-06-25 15:28 Richard Kenner
2004-06-28 15:09 ` Paolo Bonzini
2004-06-24 15:58 Richard Kenner
2004-06-24 16:04 ` Nathan Sidwell
2004-06-25 14:43 ` Joseph S. Myers
2004-06-23 5:54 Richard Kenner
2004-06-23 2:32 Richard Kenner
2004-06-23 4:54 ` Bryce McKinlay
2004-06-23 0:16 Richard Kenner
2004-06-22 23:05 Richard Kenner
2004-06-23 11:42 ` Nathan Sidwell
2004-06-22 22:18 Richard Kenner
2004-06-23 1:07 ` Richard Henderson
2004-06-22 21:30 Richard Kenner
2004-06-22 22:04 ` Paul Brook
2004-06-22 21:27 Richard Kenner
2004-06-22 21:29 ` Mark Mitchell
2004-06-22 21:10 Richard Kenner
2004-06-22 21:12 ` Mark Mitchell
2004-06-22 22:16 ` Daniel Berlin
2004-06-22 21:07 Richard Kenner
2004-06-22 21:12 ` Bryce McKinlay
2004-06-22 21:05 Richard Kenner
2004-06-22 21:01 Richard Kenner
2004-06-22 20:54 Richard Kenner
2004-06-22 21:06 ` Paul Brook
2004-06-22 21:37 ` Richard Henderson
2004-06-22 20:44 Richard Kenner
2004-06-22 21:03 ` Mark Mitchell
2004-06-23 20:58 ` Geoffrey Keating
2004-06-22 20:40 Richard Kenner
2004-06-22 22:02 ` Nathan Sidwell
2004-06-22 22:27 ` Joseph S. Myers
2004-06-22 20:23 Richard Kenner
2004-06-22 20:37 ` Joseph S. Myers
2004-06-23 20:47 ` Geoffrey Keating
2004-06-24 14:59 ` Joseph S. Myers
2004-06-24 16:26 ` Richard Earnshaw
2004-06-24 19:57 ` Laurent GUERBY
2004-06-24 20:06 ` Diego Novillo
2004-06-24 20:24 ` Andrew Pinski
2004-06-24 22:35 ` Laurent GUERBY
2004-06-24 21:33 ` Laurent GUERBY
2004-06-24 21:01 ` Joseph S. Myers
2004-06-25 14:51 ` Richard Earnshaw
2004-06-22 20:22 Richard Kenner
2004-06-22 20:30 ` Mark Mitchell
2004-06-22 19:05 Richard Kenner
2004-06-22 20:10 ` Joseph S. Myers
2004-06-22 20:27 ` Andrew Haley
[not found] <10406221359.AA05860@vlsi1.ultra.nyu.edu>
2004-06-22 18:47 ` Richard Henderson
2004-06-22 18:33 Richard Kenner
2004-06-22 18:19 Richard Kenner
2004-06-22 18:36 ` Nathan Sidwell
2004-06-22 18:37 ` Nathan Sidwell
2004-06-22 18:45 ` Mark Mitchell
2004-06-22 17:05 Richard Kenner
2004-06-22 17:21 ` Andrew Haley
2004-06-22 19:01 ` Richard Henderson
2004-06-22 16:34 Richard Kenner
2004-06-22 7:05 Richard Kenner
2004-06-22 7:29 ` Mark Mitchell
2004-06-22 9:14 ` Andrew Pinski
2004-06-22 14:16 ` Andrew Haley
2004-06-22 11:00 ` Richard Henderson
2004-06-22 13:52 ` Ranjit Mathew
2004-07-29 17:02 ` Diego Novillo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=40D857AB.1020800@codesourcery.com \
--to=mark@codesourcery.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=kenner@vlsi1.ultra.nyu.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).