On Tue, Aug 11, 2015 at 06:11:30PM -0700, Richard Henderson wrote:
> This allows testing for a mask without having to call GEN_INT.
>
> Cc: David Edelsohn <dje....@gmail.com>
> ---
> * config/rs6000/rs6000.c (rs6000_is_valid_mask_wide): Split out from...
> (rs6000_is_valid_mask): ... here.
> (rs6000_is_valid_and_mask_wide): Split out from...
> (rs6000_is_valid_and_mask): ... here.
I don't like these "_wide" names much. You could overload the shorter
name, if you really think creating some garbage const_int's is too much
overhead (it might well be if you use it a lot more in later patches).
The original functions really want rtx's since they are used like
predicates (so should look and behave like one); rs6000_is_valid_mask
itself is different (and a lousy name; suggestions welcome).
> -bool
> -rs6000_is_valid_mask (rtx mask, int *b, int *e, machine_mode mode)
> +static bool
> +rs6000_is_valid_mask_wide (unsigned HOST_WIDE_INT val, int *b, int *e, int n)
But why change the mode parameter? The code was clearer before.
> {
> - unsigned HOST_WIDE_INT val = INTVAL (mask);
> unsigned HOST_WIDE_INT bit;
> int nb, ne;
> - int n = GET_MODE_PRECISION (mode);
>
> - if (mode != DImode && mode != SImode)
> - return false;
> -
> - if (INTVAL (mask) >= 0)
> + if ((HOST_WIDE_INT)val >= 0)
^ missing space
> {
> bit = val & -val;
> ne = exact_log2 (bit);
> @@ -16430,27 +16427,54 @@ rs6000_is_valid_mask (rtx mask, int *b, int *e,
> machine_mode mode)
> return true;
> }
>
> +bool
> +rs6000_is_valid_mask (rtx mask, int *b, int *e, machine_mode mode)
> +{
> + int n;
> +
> + if (mode == DImode)
> + n = 64;
> + else if (mode == SImode)
> + n = 32;
> + else
> + return false;
> +
> + unsigned HOST_WIDE_INT val = INTVAL (mask);
> + return rs6000_is_valid_mask_wide (val, b, e, n);
> +}
> +
> /* Return whether MASK (a CONST_INT) is a valid mask for any rlwinm, rldicl,
> or rldicr instruction, to implement an AND with it in mode MODE. */
>
> -bool
> -rs6000_is_valid_and_mask (rtx mask, machine_mode mode)
> +static bool
> +rs6000_is_valid_and_mask_wide (unsigned HOST_WIDE_INT val, machine_mode mode)
> {
> int nb, ne;
>
> - if (!rs6000_is_valid_mask (mask, &nb, &ne, mode))
> - return false;
> + switch (mode)
> + {
> + case DImode:
> + if (!rs6000_is_valid_mask_wide (val, &nb, &ne, 64))
> + return false;
> + /* For DImode, we need a rldicl, rldicr, or a rlwinm with
> + mask that does not wrap. */
> + return (ne == 0 || nb == 63 || (nb < 32 && ne <= nb));
>
> - /* For DImode, we need a rldicl, rldicr, or a rlwinm with mask that
> - does not wrap. */
> - if (mode == DImode)
> - return (ne == 0 || nb == 63 || (nb < 32 && ne <= nb));
> + case SImode:
> + if (!rs6000_is_valid_mask_wide (val, &nb, &ne, 32))
> + return false;
> + /* For SImode, rlwinm can do everything. */
> + return (nb < 32 && ne < 32);
>
> - /* For SImode, rlwinm can do everything. */
> - if (mode == SImode)
> - return (nb < 32 && ne < 32);
> + default:
> + return false;
> + }
> +}
>
> - return false;
You don't need any of these changes then, either.
> +bool
> +rs6000_is_valid_and_mask (rtx mask, machine_mode mode)
> +{
> + return rs6000_is_valid_and_mask_wide (UINTVAL (mask), mode);
> }
>
> /* Return the instruction template for an AND with mask in mode MODE, with
> @@ -16739,12 +16763,12 @@ rs6000_is_valid_2insn_and (rtx c, machine_mode mode)
>
> /* Otherwise, fill in the lowest "hole"; if we can do the result with
> one insn, we can do the whole thing with two. */
> - unsigned HOST_WIDE_INT val = INTVAL (c);
> + unsigned HOST_WIDE_INT val = UINTVAL (c);
Does it matter?
Segher
