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] Fix type merging deficiency during WPA


Hi,
Sorry for jumping in late, only now I had chance to read through the whole discussion.
I was looking into similar problem some time ago.

> Index: lto-streamer-out.c
> ===================================================================
> --- lto-streamer-out.c	(revision 238156)
> +++ lto-streamer-out.c	(working copy)
> @@ -996,7 +996,9 @@ hash_tree (struct streamer_tree_cache_d
>    else
>      hstate.add_flag (TREE_NO_WARNING (t));
>    hstate.add_flag (TREE_NOTHROW (t));
> -  hstate.add_flag (TREE_STATIC (t));
> +  /* We want to unify DECL nodes in pointer fields of global types.  */
> +  if (!(VAR_OR_FUNCTION_DECL_P (t)))
> +    hstate.add_flag (TREE_STATIC (t));
>    hstate.add_flag (TREE_PROTECTED (t));
>    hstate.add_flag (TREE_DEPRECATED (t));
>    if (code != TREE_BINFO)
> @@ -1050,7 +1052,9 @@ hash_tree (struct streamer_tree_cache_d
>        hstate.add_flag (DECL_ARTIFICIAL (t));
>        hstate.add_flag (DECL_USER_ALIGN (t));
>        hstate.add_flag (DECL_PRESERVE_P (t));
> -      hstate.add_flag (DECL_EXTERNAL (t));
> +      /* We want to unify DECL nodes in pointer fields of global types.  */
> +      if (!(VAR_OR_FUNCTION_DECL_P (t)))
> +	hstate.add_flag (DECL_EXTERNAL (t));

It is fine to merge decls across static and external flags, but I am not sure this is
a safe solution to the problem. In C it is perfectly normal to have one decl more specified
or with different attributes.  Like:

extern int a __attribute__ ((warning("bar"));

int a=7 __attribute__ ((warning("foo")));

which must prevent merging otherwise the warnings will go out wrong way. In GCC
6 timeframe we decided to live with duplicated declarations and added support
for syntactic aliases. Tree merging should be optional WRT correctness, so I think
bug is in the canonical type calculation:

  /* For array types hash the domain bounds and the string flag.  */
  if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type))
    {
      hstate.add_int (TYPE_STRING_FLAG (type));
      /* OMP lowering can introduce error_mark_node in place of
         random local decls in types.  */
      if (TYPE_MIN_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
        inchash::add_expr (TYPE_MIN_VALUE (TYPE_DOMAIN (type)), hstate);
      if (TYPE_MAX_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
        inchash::add_expr (TYPE_MAX_VALUE (TYPE_DOMAIN (type)), hstate);
    }

which boils down to:

      if (tclass == tcc_declaration)                                            
        {                                                                       
          /* DECL's have a unique ID */                                         
          hstate.add_wide_int (DECL_UID (t));                                   
        }                                                                       

(in asd_expr)

It is bit ugly, but I think for static/external/public decls we need to use
assembler name here (as we can't rely on symtab to be around all the time)
which will result in unstability WRT symbol renaming and also give false
positives for static symbols. But even for static symbols it is not guaranteed
they are not duplicated.  It may be possible to use combination of assembler
name and translation unit that is available from symtab node, but not all decls
will have it defined.

Honza


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