Re: [PATCH] Fix part of pr25505

[Diego Novillo <dnovillo at redhat dot com>]

Josh Conner wrote on 08/31/06 08:53:

> -	      FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, op_iter, SSA_OP_ALL_DEFS)
> +	      if (TREE_CODE (lhs) == VAR_DECL)
>  		{
> -		  tree def = DEF_FROM_PTR (def_p);
> -		  if (TREE_CODE (def) == SSA_NAME)
> -		    def = SSA_NAME_VAR (def);
> -		  if (is_call_clobbered (def))
> +		  subvar_t subvar;
> +		  if (is_call_clobbered (lhs))
>  		    goto unsafe;
> +		  for (subvar = get_subvars_for_var (lhs);
> +		       subvar;
> +		       subvar = subvar->next)
> +		    if (is_call_clobbered (subvar->var))
> +		      goto unsafe;
> +		}
> +	      else
> +		{
> +	          FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, op_iter, SSA_OP_ALL_DEFS)
> +		    {
> +		      tree def = DEF_FROM_PTR (def_p);
> +		      if (TREE_CODE (def) == SSA_NAME)
> +		        def = SSA_NAME_VAR (def);
> +		      if (is_call_clobbered (def))
> +		        goto unsafe;
> +		    }
>  		}
>  
So, the problem here is that given

	mem_expr = clobbering_call ();

the operand scanner will give you a laundry list of all the symbols
clobbered via the VDEF list, but it will not tell you which of those
names correspond to the LHS of the assignment.  That's a limitation in
the operand scanner.  It tells you what symbols get stored by the
statement, it doesn't tell you whether they get stored by the RHS or the
LHS.

There is no easy way to fix this with the current interface, unless we
somehow added the notion of VDEF_ON_THE_LHS and VDEF_ON_THE_RHS, which
would be hideous.  Or a flag added to each VDEF entry specifying whether
it comes from the LHS or the RHS.  This last one may be doable.

On the LHS you can have an arbitrary memory expression: ARRAY_REF,
INDIRECT_REF, COMPONENT_REF, etc.  If the structure is large enough, the
compiler will not copy into a temporary.  So, to catch all the cases,
you will need to parse it.

If you only want to catch the cases VAR_DECL = CALL_EXPR, then your
logic is fine, except that you need to change the original code so that
it doesn't do unnecessary work (all the defs handed out by FOR_EACH...
are guaranteed to be SSA names).  The code that DannyB suggested should
be fine, and I think you could augment it with a type check (if the type
of the clobbered variable is not the same type as the value returned by
the call, then it can be ignored).

  FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, op_iter, SSA_OP_ALL_DEFS)
    {
      tree def = DEF_FROM_PTR (def_p);
      def = SSA_NAME_VAR (def);
      if (TREE_TYPE (def) == TREE_TYPE (call)
          && MTAG_P (def)
          && is_call_clobbered (def))
        goto unsafe;
    }

Note that I'm not 100% sure if this type check is actually valid.  But I
think it is.


>  	      /* No defs are call clobbered, so the optimization is safe.  */
> Index: gcc/tree-gimple.c
> ===================================================================
> --- gcc/tree-gimple.c	(revision 116353)
> +++ gcc/tree-gimple.c	(working copy)
> @@ -115,7 +115,9 @@ is_gimple_mem_rhs (tree t)
>       to be stored in memory, since it's cheap and prevents erroneous
>       tailcalls (PR 17526).  */
>    if (is_gimple_reg_type (TREE_TYPE (t))
> -      || TYPE_MODE (TREE_TYPE (t)) != BLKmode)
> +      || (TYPE_MODE (TREE_TYPE (t)) != BLKmode
> +	  && (TREE_CODE (t) != CALL_EXPR
> +              || ! aggregate_value_p (t, t))))
>
This is fine.