Re: try multi dest registers in default_zero_call_used_regs

[Richard Sandiford <richard.sandiford@arm.com>]

Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> When the mode of regno_reg_rtx is not hard_regno_mode_ok for the
> target, try grouping the register with subsequent ones.  This enables
> s16 to s31 and their hidden pairs to be zeroed with the default logic
> on some arm variants.
>
> Regstrapped on x86_64-linux-gnu, also tested on an affected arm
> configuration.  Ok to install?
>
>
> for  gcc/ChangeLog
>
> 	* targhooks.c (default_zero_call_used_regs): Attempt to group
> 	regs that the target refuses to use in their natural modes.

Thanks for doing this.  Some comments below…

> ---
>  gcc/targhooks.cc |   79 ++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 70 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> index fc49235eb38ee..bdaab9c63c7ee 100644
> --- a/gcc/targhooks.cc
> +++ b/gcc/targhooks.cc
> @@ -1035,16 +1035,45 @@ default_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
>      if (TEST_HARD_REG_BIT (need_zeroed_hardregs, regno))
>        {
>  	rtx_insn *last_insn = get_last_insn ();
> -	machine_mode mode = GET_MODE (regno_reg_rtx[regno]);
> +	rtx regno_rtx = regno_reg_rtx[regno];
> +	machine_mode mode = GET_MODE (regno_rtx);
> +
> +	/* If the natural mode doesn't work, try some wider mode.  */
> +	if (!targetm.hard_regno_mode_ok (regno, mode))
> +	  {
> +	    for (int nregs = 2;
> +		 regno + nregs <= FIRST_PSEUDO_REGISTER
> +		   && TEST_HARD_REG_BIT (need_zeroed_hardregs,
> +					 regno + nregs - 1);
> +		 nregs++)
> +	      {
> +		mode = choose_hard_reg_mode (regno, nregs, 0);

I like the idea, but it would be good to avoid the large:

  FIRST_PSEUDO_REGISTER * FIRST_PSEUDO_REGISTER * NUM_MACHINE_MODES

constant factor.  How about if init_reg_modes_target recorded the
maximum value of x_hard_regno_nregs?

> +		if (mode == E_VOIDmode)
> +		  continue;
> +		gcc_checking_assert (targetm.hard_regno_mode_ok (regno, mode));
> +		regno_rtx = gen_rtx_REG (mode, regno);
> +		break;
> +	      }
> +	    if (mode != GET_MODE (regno_rtx)
> +		|| regno_rtx == regno_reg_rtx[regno])
> +	      {
> +		SET_HARD_REG_BIT (failed, regno);
> +		continue;
> +	      }
> +	  }
> +
>  	rtx zero = CONST0_RTX (mode);
> -	rtx_insn *insn = emit_move_insn (regno_reg_rtx[regno], zero);
> +	rtx_insn *insn = emit_move_insn (regno_rtx, zero);
>  	if (!valid_insn_p (insn))
>  	  {
>  	    SET_HARD_REG_BIT (failed, regno);
>  	    delete_insns_since (last_insn);
>  	  }
>  	else
> -	  progress = true;
> +	  {
> +	    progress = true;
> +	    regno += hard_regno_nregs (regno, mode) - 1;
> +	  }
>        }
>  
>    /* Now retry with copies from zeroed registers, as long as we've
> @@ -1060,7 +1089,34 @@ default_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
>        for (unsigned int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
>  	if (TEST_HARD_REG_BIT (retrying, regno))
>  	  {
> -	    machine_mode mode = GET_MODE (regno_reg_rtx[regno]);
> +	    rtx regno_rtx = regno_reg_rtx[regno];
> +	    machine_mode mode = GET_MODE (regno_rtx);
> +
> +	    /* If the natural mode doesn't work, try some wider mode.  */
> +	    if (!targetm.hard_regno_mode_ok (regno, mode))
> +	      {
> +		for (int nregs = 2;
> +		     regno + nregs <= FIRST_PSEUDO_REGISTER
> +		       && TEST_HARD_REG_BIT (need_zeroed_hardregs,
> +					     regno + nregs - 1);
> +		     nregs++)
> +		  {
> +		    mode = choose_hard_reg_mode (regno, nregs, 0);
> +		    if (mode == E_VOIDmode)
> +		      continue;
> +		    gcc_checking_assert (targetm.hard_regno_mode_ok (regno,
> +								     mode));
> +		    regno_rtx = gen_rtx_REG (mode, regno);
> +		    break;
> +		  }
> +		if (mode != GET_MODE (regno_rtx)
> +		    || regno_rtx == regno_reg_rtx[regno])
> +		  {
> +		    SET_HARD_REG_BIT (failed, regno);
> +		    continue;
> +		  }
> +	      }
> +	    

This seems big enough to be worth splitting out into a helper, rather
than repeating.  That should also simplify the failure detection:
the helper can return nonnull on success and null on failure.

>  	    bool success = false;
>  	    /* Look for a source.  */
>  	    for (unsigned int src = 0; src < FIRST_PSEUDO_REGISTER; src++)
> @@ -1086,8 +1142,10 @@ default_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
>  
>  		/* SRC is usable, try to copy from it.  */
>  		rtx_insn *last_insn = get_last_insn ();
> -		rtx zsrc = gen_rtx_REG (mode, src);
> -		rtx_insn *insn = emit_move_insn (regno_reg_rtx[regno], zsrc);
> +		rtx src_rtx = (mode == GET_MODE (regno_reg_rtx[src])
> +			       ? regno_reg_rtx[src]
> +			       : gen_rtx_REG (mode, src));

Is this needed?  The original gen_rtx_REG (mode, src) seems OK.

Thanks,
Richard

> +		rtx_insn *insn = emit_move_insn (regno_rtx, src_rtx);
>  		if (!valid_insn_p (insn))
>  		  /* It didn't work, remove any inserts.  We'll look
>  		     for another SRC.  */
> @@ -1100,13 +1158,16 @@ default_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
>  		  }
>  	      }
>  
> -	    /* If nothing worked for REGNO this round, marked it to be
> +	    /* If nothing worked for REGNO this round, mark it to be
>  	       retried if we get another round.  */
>  	    if (!success)
>  	      SET_HARD_REG_BIT (failed, regno);
>  	    else
> -	      /* Take note so as to enable another round if needed.  */
> -	      progress = true;
> +	      {
> +		/* Take note so as to enable another round if needed.  */
> +		progress = true;
> +		regno += hard_regno_nregs (regno, mode) - 1;
> +	      }
>  	  }
>      }