Re: Fix representation of gcov_info_type

[Jan Hubicka <hubicka at ucw dot cz>]

> > Index: stor-layout.c
> > ===================================================================
> > --- stor-layout.c       (revision 212098)
> > +++ stor-layout.c       (working copy)
> > @@ -2065,7 +2065,7 @@ void
> >  finish_builtin_struct (tree type, const char *name, tree fields,
> >                        tree align_type)
> >  {
> > -  tree tail, next;
> > +  tree tail, next, variant;
> >
> >    for (tail = NULL_TREE; fields; tail = fields, fields = next)
> >      {
> > @@ -2074,6 +2074,10 @@ finish_builtin_struct (tree type, const
> >        DECL_CHAIN (fields) = tail;
> >      }
> >    TYPE_FIELDS (type) = tail;
> > +  for (variant = TYPE_MAIN_VARIANT (type);
> > +       variant != 0;
> > +       variant = TYPE_NEXT_VARIANT (variant))
> > +    TYPE_FIELDS (variant) = tail;
> 
> I think that's a bogus place to fix that.  Instead the caller should
> use build_variant_type_copy.  Especially that the fixup above
> depends on all variants being added before finish_builtin_struct
> is called.
> 
> Please revert the above.

Sorry, I missed this email at the airport. Will test revert overnight.

I do not understand how you propose to fix it.

The variant is produced before the builtin structure is finished and it thus
have FIELDS and SIZE NULL (as the pointer to struct itself is field of the
struct):

  /* function_info pointer pointer */
  fn_info_ptr_type = build_pointer_type
    (build_qualified_type (fn_info_ptr_type, TYPE_QUAL_CONST));
  field = build_decl (BUILTINS_LOCATION, FIELD_DECL, NULL_TREE,
                      fn_info_ptr_type);
  DECL_CHAIN (field) = fields;
  fields = field;

  finish_builtin_struct (type, "__gcov_info", fields, NULL_TREE);

which leads to build_variant_type_copy.

Once the struct is finished, we need to update the variants somewhere. Same
loop walking the variants appears in finalize_type_size that is transitively
called by finish_builtin_struct:

  /* Also layout any other variants of the type.  */
  if (TYPE_NEXT_VARIANT (type)
      || type != TYPE_MAIN_VARIANT (type))
    {
      tree variant;
      /* Record layout info of this variant.  */
      tree size = TYPE_SIZE (type);
      tree size_unit = TYPE_SIZE_UNIT (type);
      unsigned int align = TYPE_ALIGN (type);
      unsigned int user_align = TYPE_USER_ALIGN (type);
      enum machine_mode mode = TYPE_MODE (type);

      /* Copy it into all variants.  */
      for (variant = TYPE_MAIN_VARIANT (type);
           variant != 0;
           variant = TYPE_NEXT_VARIANT (variant))
        {
          TYPE_SIZE (variant) = size;
          TYPE_SIZE_UNIT (variant) = size_unit;
          TYPE_ALIGN (variant) = align;
          TYPE_USER_ALIGN (variant) = user_align;
          SET_TYPE_MODE (variant, mode);
        }
    }

Difference to my hunk is that the code works when called on non-main variant,
but I think it makes sense to always finish main variant of builtin structs.
(in fact I do not see why one would finalize size on non-main variants given
that the sizes must match either)

What would be correct fix then?
Honza


> 
> Thanks,
> Richard.
> 
> >    if (align_type)
> >      {