Re: [PATCH] New target-hook: mode_rep_extended

[Richard Sandiford <richard at codesourcery dot com>]

Adam Nemet <anemet@sonic.net> writes:
> This patch is the rework of my previous attempt to expose enough
> information about !TRULY_NOOP_TRUNCATION targets so that the
> middle-end can safely eliminate unnecessary TRUNCATE operations.
>
> The version implements Richard Sandiford's suggestion to present this
> not as a property of the TRUNCATE operation but as the property of the
> mode requiring the TRUNCATE operation.  Or with MIPS64 as an example
> we want to express this property of SImode:
>
>   (subreg:DI foo:SI 0) == (sign_extend:DI foo:SI)
>
> The patch adds a new target-hook TARGET_MODE_REP_EXTENDED(MODE,
> MODE_REP) with the meaning that if the target represents MODE
> internally as extended to MODE_REP this hook should return the type of
> the extension.

Thanks very much for all your work.  The patch looks really good to me
FWIW.  I just hope the maintainers accept it after all the hassle I've
put you through. ;)

The MIPS parts are OK to commit if everything else is approved.

Some minor nits below:

> +/* Truncation narrows the mode from SOURCE mode to DESTINATION mode.
> +   If TARGET_MODE_REP_EXTENDED (DESTINATION, DESTINATION_REP) is
> +   SIGN_EXTEND then while narrowing we also have to enforce the
> +   representation and sign-extend the value to mode DESTINATION_REP.
> +
> +   If the value is already sign-extended to DESTINATION_REP mode we
> +   can just simply switch to DESTINATION mode on it.  For each pair of
          ^^^^^^^^^^^
No "just"?

> +   integral modes SOURCE and DESTINATION, when truncating from SOURCE
> +   to DESTINATION, the table NUM_SIGN_BIT_COPIES_IN_REP contains the
> +   number of high-order bits in SOURCE that have to be copies of the
> +   sign-bit so that we can do this mode-switch to DESTINATION.  */

It isn't entirely clear from the comment whether you index the
array as num_sign_bit_copies_in_rep[SOURCE][DESTINATION] or
num_sign_bit_copies_in_rep[DESTINATION][SOURCE]

> +
> +static unsigned int
> +num_sign_bit_copies_in_rep[MAX_MODE_INT + 1][MAX_MODE_INT + 1];
>  
>  /* Return 1 if the value of X is unstable
>     (would be different at a different point in the program).
> @@ -4632,6 +4647,44 @@ get_condition (rtx jump, rtx *earliest, 
>  				 allow_cc_mode, valid_at_insn_p);
>  }
>  
> +/* Initialize the table NUM_SIGN_BIT_COPIES_IN_REP based on
> +   TARGET_MODE_REP_EXTENDED.
> +
> +   Note that we assume that the property of
> +   TARGET_MODE_REP_EXTENDED(B, C) is sticky to the integral modes
> +   narrower than mode B.  I.e., if A is a mode narrower than B then in
> +   order to be able to operate on it in mode B, mode A needs to
> +   satisfy the requirements set by the representation of mode B.  */

FWIW, you might be able to use PROMOTE_MODE to check the stickiness,
but I personally think that'd be overkill.

> +static void
> +init_num_sign_bit_copies_in_rep (void)
> +{
> +  enum machine_mode mode, in_mode;
> +
> +  for (mode = MIN_MODE_INT; mode <= MAX_MODE_INT; mode++)
> +    for (in_mode = MIN_MODE_INT; in_mode <= MAX_MODE_INT; in_mode++)
> +      {
> +	enum machine_mode i;
> +
> +	if (GET_MODE_SIZE (in_mode) <= GET_MODE_SIZE (mode))
> +	  continue;
> +
> +	for (i = mode; i != in_mode; i = GET_MODE_WIDER_MODE (i))
> +	  {
> +	    enum machine_mode wider = GET_MODE_WIDER_MODE (i);
> +
> +	    if (targetm.mode_rep_extended (i, wider) == SIGN_EXTEND
> +		/* We can only check sign-bit copies starting from the
> +		   top-bit.  In order to be able to check the ones we
> +		   have already seen we pretend that subsequent ones
> +		   have to be sign-bit copies too.  */
> +		|| num_sign_bit_copies_in_rep [mode][in_mode])
> +	      num_sign_bit_copies_in_rep [mode][in_mode]
> +		+= GET_MODE_BITSIZE (wider) - GET_MODE_BITSIZE (i);
> +	  }
> +      }
> +}

I realise that this is start-up code, so not all that performance
critical, but couldn't you avoid the cubicness with:

  for (in_mode = GET_CLASS_NARROWEST_MODE (MODE_INT); in_mode != VOIDmode;
       in_mode = GET_MODE_WIDER_MODE (in_mode))
    {
      for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT); mode != in_mode;
           mode = GET_MODE_WIDER_MODE (mode))
        if (targetm.mode_rep_extended (in_mode, mode) == SIGN_EXTEND)
          break;

      for (; mode != in_mode; mode = GET_MODE_WIDER_MODE (mode))
        num_sign_bit_copies_in_rep[mode][in_mode]
          = GET_MODE_BITSIZE (in_mode) - GET_MODE_BITSIZE (mode);
    }

(completely untested)

Richard