Only copare sizes of automatic variables

Hi,
one of common remaining reasons for ICF to fail after loading in fuction
body is mismatched type of automatic vairable.   This is becuase
compatible_types_p resorts to checking TYPE_MAIN_VARIANTS for
euqivalence that prevents merging many TBAA compaitle cases.  (And thus
is also not reflected by the hash extended by alias sets of accesses.)

Since in gimple
automatic variables are just blocks of memory I think we should only
check its size only. All accesses are matched when copmparing the actual
loads/stores.

I am not sure if we need to match types of other DECLs but I decided I can try
to be safe here: for PARM_DECl/RESUILT_DECL we match them anyway to be sure
that functions are ABI compatible.  For CONST_DECL and readonly global
VAR_DECLs they are matched when comparing their constructors.  So i think
we can keep the compare to be safe and perhaps play with it next stage1.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* ipa-icf-gimple.c (func_checker:compare_decl): Do not compare types
	of local variables.

On Fri, 20 Nov 2020, Jan Hubicka wrote:

> Hi,
> one of common remaining reasons for ICF to fail after loading in fuction
> body is mismatched type of automatic vairable.   This is becuase
> compatible_types_p resorts to checking TYPE_MAIN_VARIANTS for
> euqivalence that prevents merging many TBAA compaitle cases.  (And thus
> is also not reflected by the hash extended by alias sets of accesses.)
> 
> Since in gimple
> automatic variables are just blocks of memory I think we should only
> check its size only. All accesses are matched when copmparing the actual
> loads/stores.
> 
> I am not sure if we need to match types of other DECLs but I decided I can try
> to be safe here: for PARM_DECl/RESUILT_DECL we match them anyway to be sure
> that functions are ABI compatible.  For CONST_DECL and readonly global
> VAR_DECLs they are matched when comparing their constructors.  So i think
> we can keep the compare to be safe and perhaps play with it next stage1.
> 
> Bootstrapped/regtested x86_64-linux, OK?

I suppose we eventually check the types of SSA names?  OK if so.

Richard.

> Honza
> 
> 	* ipa-icf-gimple.c (func_checker:compare_decl): Do not compare types
> 	of local variables.
> diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
> index 69bc9ab5b34..67bb2747981 100644
> --- a/gcc/ipa-icf-gimple.c
> +++ b/gcc/ipa-icf-gimple.c
> @@ -153,8 +153,21 @@ func_checker::compare_decl (const_tree t1, const_tree t2)
>        && DECL_BY_REFERENCE (t1) != DECL_BY_REFERENCE (t2))
>      return return_false_with_msg ("DECL_BY_REFERENCE flags are different");
>  
> -  if (!compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2)))
> -    return return_false ();
> +  /* We do not really need to check types of variables, since they are just
> +     blocks of memory and we verify types of the accesses to them.
> +     However do compare types of other kinds of decls
> +     (parm decls and result decl types may affect ABI convetions).  */
> +  if (t != VAR_DECL)
> +    {
> +      if (!compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2)))
> +	return return_false ();
> +    }
> +  else
> +    {
> +      if (!operand_equal_p (DECL_SIZE (t1), DECL_SIZE (t2),
> +			    OEP_MATCH_SIDE_EFFECTS))
> +	return return_false_with_msg ("DECL_SIZEs are different");
> +    }
>  
>    bool existed_p;
>    const_tree &slot = m_decl_map.get_or_insert (t1, &existed_p);
>

> On Fri, 20 Nov 2020, Jan Hubicka wrote:
> 
> > Hi,
> > one of common remaining reasons for ICF to fail after loading in fuction
> > body is mismatched type of automatic vairable.   This is becuase
> > compatible_types_p resorts to checking TYPE_MAIN_VARIANTS for
> > euqivalence that prevents merging many TBAA compaitle cases.  (And thus
> > is also not reflected by the hash extended by alias sets of accesses.)
> > 
> > Since in gimple
> > automatic variables are just blocks of memory I think we should only
> > check its size only. All accesses are matched when copmparing the actual
> > loads/stores.
> > 
> > I am not sure if we need to match types of other DECLs but I decided I can try
> > to be safe here: for PARM_DECl/RESUILT_DECL we match them anyway to be sure
> > that functions are ABI compatible.  For CONST_DECL and readonly global
> > VAR_DECLs they are matched when comparing their constructors.  So i think
> > we can keep the compare to be safe and perhaps play with it next stage1.
> > 
> > Bootstrapped/regtested x86_64-linux, OK?
> 
> I suppose we eventually check the types of SSA names?  OK if so.

Yes, SSA names are checked as part of operand_equal_p.  So VAR_DECLs of
register variables are safe too (as are SSA_NAMES with no associated
VAR_DECL)

Honza