This is the mail archive of the gcc-patches@gcc.gnu.org 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] support static nested constructors in bitfields


Olivier Hainque <hainque@adacore.com> writes:

> 1/ > Test against NULL_TREE, not 0 (I know the existing code uses 0).
>
> What about the cases where the comparison isn't explicit, for
> instance:
>
>       if (!outer
> 	  && local.index && TREE_CODE (local.index) == RANGE_EXPR)
>              ^^^^^^^^^^^
>
> are these fine or do you prefer an explicit '!= NULL_TREE' added ?

Either approach is acceptable.  I personally prefer the explicit !=
NULL_TREE, but certainly many others disagree.  There is no need to add
it where it is not present.


> 2/ For Ada, a piece of gigi needs a tight synchronization with one
> of the internal predicates in output_constructor: the "is this a
> true bitfield" part of
>
>       /* Output the current element, using the appropriate helper ...  */
>       ....
>       /* For a field that is neither a true bitfield nor part of an outer one,
> 	 known to be at least byte aligned and multiple-of-bytes long.  */
>         else if (!outer
>                && (local.field == NULL_TREE
>                    || !DECL_BIT_FIELD (local.field)
>                    || DECL_MODE (local.field) == BLKmode))
>
> We have the attached patchlet to help there.
>
>         * tree.h (CONSTRUCTOR_BITFIELD_P): True if NODE, a FIELD_DECL, is
>         to be processed as a bitfield for constructor output purposes.
>         * varasm.c (output_constructor): Use it.
>
> May I include this as well ?


> + /* True if NODE, a FIELD_DECL, is to be processed as a bitfield for
> +    constructor output purposes.  */
> + #define CONSTRUCTOR_BITFIELD_P(NODE) \
> +   ((DECL_BIT_FIELD (FIELD_DECL_CHECK (NODE)) && DECL_MODE (NODE) != BLKmode))

You have double parentheses around the whole expression; drop the outer
ones.


> *** gcc/varasm.c.ori	Thu Oct 30 11:15:22 2008
> --- gcc/varasm.c	Fri Oct 31 16:03:24 2008
> *************** output_constructor (tree exp, unsigned H
> *** 5069,5077 ****
>         /* For a field that is neither a true bitfield nor part of an outer one,
>   	 known to be at least byte aligned and multiple-of-bytes long.  */
>         else if (!outer
> ! 	       && (local.field == NULL_TREE
> ! 		   || !DECL_BIT_FIELD (local.field)
> ! 		   || DECL_MODE (local.field) == BLKmode))
>   	output_constructor_regular_field (&local);
        
>         /* For a true bitfield or part of an outer one.  */
> --- 5069,5075 ----
>         /* For a field that is neither a true bitfield nor part of an outer one,
>   	 known to be at least byte aligned and multiple-of-bytes long.  */
>         else if (!outer
> ! 	       && (local.field == 0 || !CONSTRUCTOR_BITFIELD_P (local.field)))
>   	output_constructor_regular_field (&local);
        
>         /* For a true bitfield or part of an outer one.  */

S/0/NULL_TREE/ (you're going backward!)

This is OK.

Thanks.

Ian


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