Re: [ping] three patch suggestions

[Ian Lance Taylor <iant at google dot com>]

Olivier Hainque <hainque@adacore.com> writes:

> Ping for three patch submissions summarized below, all addressing regressions
> from the GCC 3.4 series on ix86-linux. The failures are still visible with
> current mainline. I'll be happy to repeat the full testing process again if
> the changes are considered appropriate.

>    prevent creation of improper static vars in gimplify_init_constructor
>    http://gcc.gnu.org/ml/gcc-patches/2006-05/msg00450.html

The issue I see with this patch is that we are going to wind up
walking the entire constructor twice: once in categorize_ctor_elements
and once in initializer_constant_valid_p.  This could be a bit painful
for very large initializers, which is a case which we already handle
poorly.

It seems clear that output_constructor could handle this case
(basically a struct in a bitfield), but it would be kind of
complicated, and since this case can not arise for C or C++ I don't
care very much about it.

As far as I can see, the reason that your patch works, the reason that
initializer_constant_valid_p returns false, is that TREE_CONSTANT is
not set for the constructor, even though it is a constant.  If
TREE_CONSTANT were set, initializer_constant_valid_p would walk the
elements and report that they were all constant.

I haven't looked into why TREE_CONSTANT is not set, but I assume it
has something to do with the Ada frontend.  In any case it suggests a
simple fix: change categorize_ctor_elements_1 so that when it sees a
constructor for which TREE_CONSTANT is not set, it increments nc_elts.
That is, something like this:

	case CONSTRUCTOR:
	  {
	    HOST_WIDE_INT nz = 0, nc = 0, ic = 0;
	    categorize_ctor_elements_1 (value, &nz, &nc, &ic, p_must_clear);
	    nz_elts += mult * nz;

	    /* We check TREE_CONSTANT here so that we are compatible
	       with initializer_constant_valid_p.  We don't want to
	       return nc_elts == 0 when initializer_constant_valid_p
	       will not return NULL_TREE.  FIXME: The underlying issue
	       is that Ada (but not C/C++) can produce constant
	       initialized bitfield aggregates which
	       output_constructor can not handle.  These initializers
	       are constant, but TREE_CONSTANT is not set.  A better
	       overall fix would be to set TREE_CONSTANT correctly and
	       to handle this case in output_constructor.  */
	    if (TREE_CONSTANT (value))
	      nc_elts += mult * nc;
	    else
	      nc_elts += mult * ic;

	    elt_count += mult * ic;
	  }
	  break;

A patch along these lines is preapproved.  Please make sure that there
is a test case in the Ada testsuite.

Thanks.

Ian