This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch to allow Ada to work with tree-ssa

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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]