Re: [PATCH][4.8] C++ memory model bitfield handling rewrite

[Richard Guenther <rguenther at suse dot de>]

On Tue, 7 Feb 2012, Joseph S. Myers wrote:

> On Tue, 7 Feb 2012, Richard Guenther wrote:
> 
> > If the C frontend would stop using DECL_INITIAL temporarily for
> > bitfield FIELD_DECLs we could avoid adding a new member to struct
> > tree_field_decl - Joseph, is it possible to avoid using DECL_INITIAL?
> 
> C++ does the same thing with DECL_INITIAL so I'd expect that to need to 
> change as well - Jason?  C only needs an integer width within a limited 
> range; C++ may actually need a general expression here.
> 
> C currently uses a TREE_LIST of field declarations when parsing a 
> structure.  It should be reasonably straightforward to change that to a 
> VEC of structures storing both the declaration and the width, so the width 
> no longer need go in DECL_INITIAL of the declaration, though it will be 
> necessary to check for other code using DECL_INITIAL to check for 
> bit-fields.  For example, c-common.c:handle_packed_attribute uses 
> DECL_INITIAL like that; it should be possible to make both C and C++ set 
> DECL_C_BIT_FIELD early enough that such DECL_INITIAL checks can be 
> replaced by a more meaningful DECL_C_BIT_FIELD check.

That would be nice.  I suppose that doing

      if (DECL_INITIAL (x))
        {
          unsigned HOST_WIDE_INT width = tree_low_cst (DECL_INITIAL (x), 
1);
          DECL_SIZE (x) = bitsize_int (width);
          DECL_BIT_FIELD (x) = 1;
          SET_DECL_C_BIT_FIELD (x);
        }

at the time we set DECL_INITIAL to the width is not possible
for some weird reason?  At least the struct-layout-1.exp tests
seem to be happy with

@@ -6808,6 +6837,12 @@ grokfield (location_t loc,
 
   finish_decl (value, loc, NULL_TREE, NULL_TREE, NULL_TREE);
   DECL_INITIAL (value) = width;
+  if (width)
+    {
+      DECL_SIZE (value) = bitsize_int (tree_low_cst (width, 1));
+      DECL_BIT_FIELD (value) = 1;
+      SET_DECL_C_BIT_FIELD (value);
+    }
 
   if (warn_cxx_compat && DECL_NAME (value) != NULL_TREE)
     {
@@ -7132,14 +7167,6 @@ finish_struct (location_t loc, tree t, t
       if (C_DECL_VARIABLE_SIZE (x))
        C_TYPE_VARIABLE_SIZE (t) = 1;
 
-      if (DECL_INITIAL (x))
-       {
-         unsigned HOST_WIDE_INT width = tree_low_cst (DECL_INITIAL (x), 
1);
-         DECL_SIZE (x) = bitsize_int (width);
-         DECL_BIT_FIELD (x) = 1;
-         SET_DECL_C_BIT_FIELD (x);
-       }
-
       if (TYPE_PACKED (t)
          && (DECL_BIT_FIELD (x)
              || TYPE_ALIGN (TREE_TYPE (x)) > BITS_PER_UNIT))
@@ -7195,11 +7222,12 @@ finish_struct (location_t loc, tree t, t
   {
     tree *fieldlistp = &fieldlist;
     while (*fieldlistp)
-      if (TREE_CODE (*fieldlistp) == FIELD_DECL && DECL_INITIAL 
(*fieldlistp)
+      if (TREE_CODE (*fieldlistp) == FIELD_DECL
+         && DECL_C_BIT_FIELD (*fieldlistp)
          && TREE_TYPE (*fieldlistp) != error_mark_node)
        {
          unsigned HOST_WIDE_INT width
-           = tree_low_cst (DECL_INITIAL (*fieldlistp), 1);
+           = tree_low_cst (DECL_SIZE (*fieldlistp), 1);
          tree type = TREE_TYPE (*fieldlistp);
          if (width != TYPE_PRECISION (type))
            {
@@ -7207,7 +7235,6 @@ finish_struct (location_t loc, tree t, t
                = c_build_bitfield_integer_type (width, TYPE_UNSIGNED 
(type));
              DECL_MODE (*fieldlistp) = TYPE_MODE (TREE_TYPE 
(*fieldlistp));
            }
-         DECL_INITIAL (*fieldlistp) = 0;
        }
       else
        fieldlistp = &DECL_CHAIN (*fieldlistp);

it remains of course to replace checks for bitfieldness with
DECL_C_BIT_FIELD checks.  stor-layout at least doesn't seem to
mess with DECL_SIZE unless it is NULL (unless you happen to
call relayout_decl on a FIELD_DECL which I think the C FE does 
not do for bitfields)

Richard.