From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31890 invoked by alias); 22 Jun 2004 16:00:43 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 31749 invoked from network); 22 Jun 2004 16:00:41 -0000 Received: from unknown (HELO mail.codesourcery.com) (65.74.133.5) by sourceware.org with SMTP; 22 Jun 2004 16:00:41 -0000 Received: (qmail 19471 invoked from network); 22 Jun 2004 16:00:40 -0000 Received: from localhost (HELO codesourcery.com) (mitchell@127.0.0.1) by mail.codesourcery.com with SMTP; 22 Jun 2004 16:00:40 -0000 Message-ID: <40D857AB.1020800@codesourcery.com> Date: Tue, 22 Jun 2004 17:46:00 -0000 From: Mark Mitchell Organization: CodeSourcery, LLC User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040113 MIME-Version: 1.0 To: Richard Kenner CC: gcc-patches@gcc.gnu.org Subject: Re: Patch to allow Ada to work with tree-ssa References: <10406221307.AA05385@vlsi1.ultra.nyu.edu> In-Reply-To: <10406221307.AA05385@vlsi1.ultra.nyu.edu> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-06/txt/msg01755.txt.bz2 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