[SVE] [fwprop] PR88833 - Redundant moves for WHILELO-based loops

Richard Sandiford richard.sandiford@arm.com
Mon Jun 24 09:29:00 GMT 2019


Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> diff --git a/gcc/fwprop.c b/gcc/fwprop.c
> index 45703fe5f01..93a1a10c9a6 100644
> --- a/gcc/fwprop.c
> +++ b/gcc/fwprop.c
> @@ -547,6 +547,54 @@ propagate_rtx_1 (rtx *px, rtx old_rtx, rtx new_rtx, int flags)
>  	  tem = simplify_gen_subreg (mode, op0, GET_MODE (SUBREG_REG (x)),
>  				     SUBREG_BYTE (x));
>  	}
> +
> +      else
> +	{
> +	  rtvec vec;
> +	  rtvec newvec;
> +	  const char *fmt = GET_RTX_FORMAT (code);
> +	  rtx op;
> +
> +	  for (int i = 0; fmt[i]; i++)
> +	    switch (fmt[i])
> +	      {
> +	      case 'E':
> +		vec = XVEC (x, i);
> +		newvec = vec;
> +		for (int j = 0; j < GET_NUM_ELEM (vec); j++)
> +		  {
> +		    op = RTVEC_ELT (vec, j);
> +		    valid_ops &= propagate_rtx_1 (&op, old_rtx, new_rtx, flags);
> +		    if (op != RTVEC_ELT (vec, j))
> +		      {
> +			if (newvec == vec)
> +			  {
> +			    newvec = shallow_copy_rtvec (vec);
> +			    if (!tem)
> +			      tem = shallow_copy_rtx (x);
> +			    XVEC (tem, i) = newvec;
> +			  }
> +			RTVEC_ELT (newvec, j) = op;
> +		      }
> +		  }
> +	      break;

Misindented break: should be indented two columns further.

> +
> +	      case 'e':
> +		if (XEXP (x, i))
> +		  {
> +		    op = XEXP (x, i);
> +		    valid_ops &= propagate_rtx_1 (&op, old_rtx, new_rtx, flags);
> +		    if (op != XEXP (x, i))
> +		      {
> +			if (!tem)
> +			  tem = shallow_copy_rtx (x);
> +			XEXP (tem, i) = op;
> +		      }
> +		  }
> +	      break;

Same here.

> +	      }
> +	}
> +
>        break;
>  
>      case RTX_OBJ:
> @@ -1370,10 +1418,11 @@ forward_propagate_and_simplify (df_ref use, rtx_insn *def_insn, rtx def_set)
>  
>  /* Given a use USE of an insn, if it has a single reaching
>     definition, try to forward propagate it into that insn.
> -   Return true if cfg cleanup will be needed.  */
> +   Return true if cfg cleanup will be needed.
> +   REG_PROP_ONLY is true if we should only propagate register copies.  */
>  
>  static bool
> -forward_propagate_into (df_ref use)
> +forward_propagate_into (df_ref use, bool reg_prop_only = false)
>  {
>    df_ref def;
>    rtx_insn *def_insn, *use_insn;
> @@ -1394,10 +1443,6 @@ forward_propagate_into (df_ref use)
>    if (DF_REF_IS_ARTIFICIAL (def))
>      return false;
>  
> -  /* Do not propagate loop invariant definitions inside the loop.  */
> -  if (DF_REF_BB (def)->loop_father != DF_REF_BB (use)->loop_father)
> -    return false;
> -
>    /* Check if the use is still present in the insn!  */
>    use_insn = DF_REF_INSN (use);
>    if (DF_REF_FLAGS (use) & DF_REF_IN_NOTE)
> @@ -1415,6 +1460,16 @@ forward_propagate_into (df_ref use)
>    if (!def_set)
>      return false;
>  
> +  if (reg_prop_only && !REG_P (SET_SRC (def_set)))
> +    return false;
> +
> +  /* Allow propagating def inside loop only if source of def_set is
> +     reg, since replacing reg by source reg shouldn't increase cost.  */

Maybe:

  /* Allow propagations into a loop only for reg-to-reg copies, since
     replacing one register by another shouldn't increase the cost.  */

> +
> +  if (DF_REF_BB (def)->loop_father != DF_REF_BB (use)->loop_father
> +      && !REG_P (SET_SRC (def_set)))
> +    return false;

To be extra safe, I think we should check that the SET_DEST is a REG_P
in both cases, to exclude REG-to-SUBREG copies.

Thanks,
Richard



More information about the Gcc-patches mailing list