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

[kenner@vlsi1.ultra.nyu.edu (Richard Kenner)]

    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.

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.

    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.

    You also didn't add accessor macros for these additional operands, so
    the only place to look is at the documentation you added in tree.def,
    which suggests that these additional operands are redundant; 

I added accessor *functions* for each of these:
	array_ref_low_bound,
	array_ref_element_size, and
	component_ref_field_offset
They are supposed to be used (and are used) whenever those fields
are needed.

We don't have accessor macros for any other fields of references or
expressions, so I didn't add them for this.  I would be in favor of
adding accesor macros for all expressions (e.g. MODIFY_EXPR_RHS) to
include these, but I think it would be very peculiar to have an
accessor macro for operands 2 and 3 of an ARRAY_REF when we don't have
them for operands 0 and 1!  That's why I didn't add them.

    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.

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.

    If that's what this is about, then I suggest you find a way to make
    these nodes bigger in Ada than it is in C/C++ and only look for the
    additional fields there, or have a flag in ARRAY_REF that indicates
    whether these additional fields are present.

It's not C vs. Ada but fixed vs. variable.  Most arrays and components in Ada
are fixed size, just like in C.  It's more an issue of *frequency*.  In C and
C++, variable-sized stuff is quite rare, while in Ada it's more common.

We don't have a mechanism to have a tree code be of two different sizes
depending on the setting of a flag, so we'd have to add one.  We could have
two different tree codes, but that would complicate the optimizers and is
exactly what this mechanism is trying to avoid.

    Please revert this patch until we've got consensus here.

I don't want to do that.  It's a large patch and I have further changes that
I'll be making on top of it and reverting it at this point would bring my
work to a stop for an indefinite amount of time.

We *had* concensus.  This approach was discussed *weeks* ago and nobody
objected, including you.  I wouldn't have spent weeks implementing and
testing this if I didn't feel everybody agreed on it.

Look, for example, at http://gcc.gnu.org/ml/gcc/2004-06/msg00459.html
That was over two weeks ago and was the time to object.

Yes, it adds some more memory utilization, and yes, that's not good.  But
it's very small. According to Nathan and Zack's paper, expression nodes in
total amount to 19% of memory utilization in C++ programs.

An expression node has quite a large number of fields.  The common part is
four words.  The expression adds another four words plus the operands.  So
ARRAY_REF used to be 12 words and is now 14, an increase of 17%.
COMPONENT_REF used to be 12 and is now 13, an increase of 8%.  I don't know
how common those nodes are in expressions, but in GIMPLE, almost half of all
expr nodes are MODIFY_EXPR.

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.

Nathan and Zack are working on redoing the tree structure anyway so this can
get taken care of then.

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.