[5/9] Add mode_for_int_vector helper functions

There are at least a few places that want to create an integer vector
with a specified element size and element count, or to create the
integer equivalent of an existing mode.  This patch adds helpers
for doing that.

The require ()s are all used in functions that go on to emit
instructions that use the result as a vector mode.

2017-09-04  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* machmode.h (mode_for_int_vector): New function.
	* stor-layout.c (mode_for_int_vector): Likewise.
	* config/aarch64/aarch64.c (aarch64_emit_approx_sqrt): Use it.
	* config/powerpcspe/powerpcspe.c (rs6000_do_expand_vec_perm): Likewise.
	* config/rs6000/rs6000.c (rs6000_do_expand_vec_perm): Likewise.
	* config/s390/s390.c (s390_expand_vec_compare_cc): Likewise.
	(s390_expand_vcond): Likewise.

On Mon, Sep 4, 2017 at 1:36 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> There are at least a few places that want to create an integer vector
> with a specified element size and element count, or to create the
> integer equivalent of an existing mode.  This patch adds helpers
> for doing that.
>
> The require ()s are all used in functions that go on to emit
> instructions that use the result as a vector mode.
>
> 2017-09-04  Richard Sandiford  <richard.sandiford@linaro.org>
>
> gcc/
>         * machmode.h (mode_for_int_vector): New function.
>         * stor-layout.c (mode_for_int_vector): Likewise.
>         * config/aarch64/aarch64.c (aarch64_emit_approx_sqrt): Use it.
>         * config/powerpcspe/powerpcspe.c (rs6000_do_expand_vec_perm): Likewise.
>         * config/rs6000/rs6000.c (rs6000_do_expand_vec_perm): Likewise.
>         * config/s390/s390.c (s390_expand_vec_compare_cc): Likewise.
>         (s390_expand_vcond): Likewise.
>
> Index: gcc/machmode.h
> ===================================================================
> --- gcc/machmode.h      2017-09-04 12:18:50.674859598 +0100
> +++ gcc/machmode.h      2017-09-04 12:18:53.153306182 +0100
> @@ -706,6 +706,21 @@ extern machine_mode bitwise_mode_for_mod
>
>  extern machine_mode mode_for_vector (scalar_mode, unsigned);
>
> +extern opt_machine_mode mode_for_int_vector (unsigned int, unsigned int);
> +
> +/* Return the integer vector equivalent of MODE, if one exists.  In other
> +   words, return the mode for an integer vector that has the same number
> +   of bits as MODE and the same number of elements as MODE, with the
> +   latter being 1 if MODE is scalar.  The returned mode can be either
> +   an integer mode or a vector mode.  */
> +
> +inline opt_machine_mode
> +mode_for_int_vector (machine_mode mode)

So this is similar to int_mode_for_mode which means...

int_vector_mode_for_vector_mode?

> +{

Nothing prevents use with non-vector MODE here, can we place an assert here?

> +  return mode_for_int_vector (GET_MODE_UNIT_BITSIZE (mode),
> +                             GET_MODE_NUNITS (mode));
> +}
> +
>  /* A class for iterating through possible bitfield modes.  */
>  class bit_field_mode_iterator
>  {
> Index: gcc/stor-layout.c
> ===================================================================
> --- gcc/stor-layout.c   2017-09-04 12:18:50.675762071 +0100
> +++ gcc/stor-layout.c   2017-09-04 12:18:53.153306182 +0100
> @@ -517,6 +517,23 @@ mode_for_vector (scalar_mode innermode,
>    return mode;
>  }
>
> +/* Return the mode for a vector that has NUNITS integer elements of
> +   INT_BITS bits each, if such a mode exists.  The mode can be either
> +   an integer mode or a vector mode.  */
> +
> +opt_machine_mode
> +mode_for_int_vector (unsigned int int_bits, unsigned int nunits)

That's more vector_int_mode_for_size (...), no?  Similar to int_mode_for_size
or mode_for_size.

Ok with those renamings.  I wonder if int_vector_mode_for_vector_mode
is necessary -- is calling vector_int_mode_for_size
(GET_MODE_UNIT_BITSIZE (mode),
GET_MODE_NUNITS (mode)) too cumbersome?

> +{
> +  scalar_int_mode int_mode;
> +  if (int_mode_for_size (int_bits, 0).exists (&int_mode))
> +    {
> +      machine_mode vec_mode = mode_for_vector (int_mode, nunits);

Uh, so we _do_ have an existing badly named 'mode_for_vector' ...

> +      if (vec_mode != BLKmode)
> +       return vec_mode;
> +    }
> +  return opt_machine_mode ();
> +}
> +
>  /* Return the alignment of MODE. This will be bounded by 1 and
>     BIGGEST_ALIGNMENT.  */
>
> Index: gcc/config/aarch64/aarch64.c
> ===================================================================
> --- gcc/config/aarch64/aarch64.c        2017-09-04 12:18:44.874165502 +0100
> +++ gcc/config/aarch64/aarch64.c        2017-09-04 12:18:53.144272229 +0100
> @@ -8282,9 +8282,6 @@ aarch64_emit_approx_sqrt (rtx dst, rtx s
>        return false;
>      }
>
> -  machine_mode mmsk
> -    = mode_for_vector (int_mode_for_mode (GET_MODE_INNER (mode)).require (),
> -                      GET_MODE_NUNITS (mode));
>    if (!recp)
>      {
>        if (!(flag_mlow_precision_sqrt
> @@ -8302,7 +8299,7 @@ aarch64_emit_approx_sqrt (rtx dst, rtx s
>      /* Caller assumes we cannot fail.  */
>      gcc_assert (use_rsqrt_p (mode));
>
> -
> +  machine_mode mmsk = mode_for_int_vector (mode).require ();
>    rtx xmsk = gen_reg_rtx (mmsk);
>    if (!recp)
>      /* When calculating the approximate square root, compare the
> Index: gcc/config/powerpcspe/powerpcspe.c
> ===================================================================
> --- gcc/config/powerpcspe/powerpcspe.c  2017-09-04 12:18:44.919414816 +0100
> +++ gcc/config/powerpcspe/powerpcspe.c  2017-09-04 12:18:53.148287319 +0100
> @@ -38739,8 +38739,7 @@ rs6000_do_expand_vec_perm (rtx target, r
>
>    imode = vmode;
>    if (GET_MODE_CLASS (vmode) != MODE_VECTOR_INT)
> -    imode = mode_for_vector
> -      (int_mode_for_mode (GET_MODE_INNER (vmode)).require (), nelt);
> +    imode = mode_for_int_vector (vmode).require ();
>
>    x = gen_rtx_CONST_VECTOR (imode, gen_rtvec_v (nelt, perm));
>    x = expand_vec_perm (vmode, op0, op1, x, target);
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  2017-09-04 12:18:44.929470219 +0100
> +++ gcc/config/rs6000/rs6000.c  2017-09-04 12:18:53.151298637 +0100
> @@ -35584,8 +35584,7 @@ rs6000_do_expand_vec_perm (rtx target, r
>
>    imode = vmode;
>    if (GET_MODE_CLASS (vmode) != MODE_VECTOR_INT)
> -    imode = mode_for_vector
> -      (int_mode_for_mode (GET_MODE_INNER (vmode)).require (), nelt);
> +    imode = mode_for_int_vector (vmode).require ();
>
>    x = gen_rtx_CONST_VECTOR (imode, gen_rtvec_v (nelt, perm));
>    x = expand_vec_perm (vmode, op0, op1, x, target);
> Index: gcc/config/s390/s390.c
> ===================================================================
> --- gcc/config/s390/s390.c      2017-09-04 11:50:24.561571751 +0100
> +++ gcc/config/s390/s390.c      2017-09-04 12:18:53.153306182 +0100
> @@ -6472,10 +6472,7 @@ s390_expand_vec_compare_cc (rtx target,
>         case LE:   cc_producer_mode = CCVFHEmode; code = GE; swap_p = true; break;
>         default: gcc_unreachable ();
>         }
> -      scratch_mode = mode_for_vector
> -       (int_mode_for_mode (GET_MODE_INNER (GET_MODE (cmp1))).require (),
> -        GET_MODE_NUNITS (GET_MODE (cmp1)));
> -      gcc_assert (scratch_mode != BLKmode);
> +      scratch_mode = mode_for_int_vector (GET_MODE (cmp1)).require ();
>
>        if (inv_p)
>         all_p = !all_p;
> @@ -6581,9 +6578,7 @@ s390_expand_vcond (rtx target, rtx then,
>
>    /* We always use an integral type vector to hold the comparison
>       result.  */
> -  result_mode = mode_for_vector
> -    (int_mode_for_mode (GET_MODE_INNER (cmp_mode)).require (),
> -     GET_MODE_NUNITS (cmp_mode));
> +  result_mode = mode_for_int_vector (cmp_mode).require ();
>    result_target = gen_reg_rtx (result_mode);
>
>    /* We allow vector immediates as comparison operands that

Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, Sep 4, 2017 at 1:36 PM, Richard Sandiford
> <richard.sandiford@linaro.org> wrote:
>> There are at least a few places that want to create an integer vector
>> with a specified element size and element count, or to create the
>> integer equivalent of an existing mode.  This patch adds helpers
>> for doing that.
>>
>> The require ()s are all used in functions that go on to emit
>> instructions that use the result as a vector mode.
>>
>> 2017-09-04  Richard Sandiford  <richard.sandiford@linaro.org>
>>
>> gcc/
>>         * machmode.h (mode_for_int_vector): New function.
>>         * stor-layout.c (mode_for_int_vector): Likewise.
>>         * config/aarch64/aarch64.c (aarch64_emit_approx_sqrt): Use it.
>>         * config/powerpcspe/powerpcspe.c (rs6000_do_expand_vec_perm): Likewise.
>>         * config/rs6000/rs6000.c (rs6000_do_expand_vec_perm): Likewise.
>>         * config/s390/s390.c (s390_expand_vec_compare_cc): Likewise.
>>         (s390_expand_vcond): Likewise.
>>
>> Index: gcc/machmode.h
>> ===================================================================
>> --- gcc/machmode.h      2017-09-04 12:18:50.674859598 +0100
>> +++ gcc/machmode.h      2017-09-04 12:18:53.153306182 +0100
>> @@ -706,6 +706,21 @@ extern machine_mode bitwise_mode_for_mod
>>
>>  extern machine_mode mode_for_vector (scalar_mode, unsigned);
>>
>> +extern opt_machine_mode mode_for_int_vector (unsigned int, unsigned int);
>> +
>> +/* Return the integer vector equivalent of MODE, if one exists.  In other
>> +   words, return the mode for an integer vector that has the same number
>> +   of bits as MODE and the same number of elements as MODE, with the
>> +   latter being 1 if MODE is scalar.  The returned mode can be either
>> +   an integer mode or a vector mode.  */
>> +
>> +inline opt_machine_mode
>> +mode_for_int_vector (machine_mode mode)
>
> So this is similar to int_mode_for_mode which means...
>
> int_vector_mode_for_vector_mode?

I'd used that style of name originally, but didn't like it because
it gave the impression that the result would be a VECTOR_MODE_P.

mode_for_int_vector was supposed to be consistent with mode_for_vector.

>> +{
>
> Nothing prevents use with non-vector MODE here, can we place an assert here?

That was deliberate.  I wanted it to work with scalars too, returning
a V1xx in that case.

>> +  return mode_for_int_vector (GET_MODE_UNIT_BITSIZE (mode),
>> +                             GET_MODE_NUNITS (mode));
>> +}
>> +
>>  /* A class for iterating through possible bitfield modes.  */
>>  class bit_field_mode_iterator
>>  {
>> Index: gcc/stor-layout.c
>> ===================================================================
>> --- gcc/stor-layout.c   2017-09-04 12:18:50.675762071 +0100
>> +++ gcc/stor-layout.c   2017-09-04 12:18:53.153306182 +0100
>> @@ -517,6 +517,23 @@ mode_for_vector (scalar_mode innermode,
>>    return mode;
>>  }
>>
>> +/* Return the mode for a vector that has NUNITS integer elements of
>> +   INT_BITS bits each, if such a mode exists.  The mode can be either
>> +   an integer mode or a vector mode.  */
>> +
>> +opt_machine_mode
>> +mode_for_int_vector (unsigned int int_bits, unsigned int nunits)
>
> That's more vector_int_mode_for_size (...), no?  Similar to int_mode_for_size
> or mode_for_size.
>
> Ok with those renamings.  I wonder if int_vector_mode_for_vector_mode
> is necessary -- is calling vector_int_mode_for_size
> (GET_MODE_UNIT_BITSIZE (mode),
> GET_MODE_NUNITS (mode)) too cumbersome?

IMO yes :-)  It's certainly longer than the equivalent int_mode_for_mode
expansion.

Thanks,
Richard

On Tue, Sep 5, 2017 at 2:33 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, Sep 4, 2017 at 1:36 PM, Richard Sandiford
>> <richard.sandiford@linaro.org> wrote:
>>> There are at least a few places that want to create an integer vector
>>> with a specified element size and element count, or to create the
>>> integer equivalent of an existing mode.  This patch adds helpers
>>> for doing that.
>>>
>>> The require ()s are all used in functions that go on to emit
>>> instructions that use the result as a vector mode.
>>>
>>> 2017-09-04  Richard Sandiford  <richard.sandiford@linaro.org>
>>>
>>> gcc/
>>>         * machmode.h (mode_for_int_vector): New function.
>>>         * stor-layout.c (mode_for_int_vector): Likewise.
>>>         * config/aarch64/aarch64.c (aarch64_emit_approx_sqrt): Use it.
>>>         * config/powerpcspe/powerpcspe.c (rs6000_do_expand_vec_perm): Likewise.
>>>         * config/rs6000/rs6000.c (rs6000_do_expand_vec_perm): Likewise.
>>>         * config/s390/s390.c (s390_expand_vec_compare_cc): Likewise.
>>>         (s390_expand_vcond): Likewise.
>>>
>>> Index: gcc/machmode.h
>>> ===================================================================
>>> --- gcc/machmode.h      2017-09-04 12:18:50.674859598 +0100
>>> +++ gcc/machmode.h      2017-09-04 12:18:53.153306182 +0100
>>> @@ -706,6 +706,21 @@ extern machine_mode bitwise_mode_for_mod
>>>
>>>  extern machine_mode mode_for_vector (scalar_mode, unsigned);
>>>
>>> +extern opt_machine_mode mode_for_int_vector (unsigned int, unsigned int);
>>> +
>>> +/* Return the integer vector equivalent of MODE, if one exists.  In other
>>> +   words, return the mode for an integer vector that has the same number
>>> +   of bits as MODE and the same number of elements as MODE, with the
>>> +   latter being 1 if MODE is scalar.  The returned mode can be either
>>> +   an integer mode or a vector mode.  */
>>> +
>>> +inline opt_machine_mode
>>> +mode_for_int_vector (machine_mode mode)
>>
>> So this is similar to int_mode_for_mode which means...
>>
>> int_vector_mode_for_vector_mode?
>
> I'd used that style of name originally, but didn't like it because
> it gave the impression that the result would be a VECTOR_MODE_P.

Oh, it isn't?  Ah, yes, it can be an integer mode...

> mode_for_int_vector was supposed to be consistent with mode_for_vector.
>
>>> +{
>>
>> Nothing prevents use with non-vector MODE here, can we place an assert here?
>
> That was deliberate.  I wanted it to work with scalars too, returning
> a V1xx in that case.

Ok.

>>> +  return mode_for_int_vector (GET_MODE_UNIT_BITSIZE (mode),
>>> +                             GET_MODE_NUNITS (mode));
>>> +}
>>> +
>>>  /* A class for iterating through possible bitfield modes.  */
>>>  class bit_field_mode_iterator
>>>  {
>>> Index: gcc/stor-layout.c
>>> ===================================================================
>>> --- gcc/stor-layout.c   2017-09-04 12:18:50.675762071 +0100
>>> +++ gcc/stor-layout.c   2017-09-04 12:18:53.153306182 +0100
>>> @@ -517,6 +517,23 @@ mode_for_vector (scalar_mode innermode,
>>>    return mode;
>>>  }
>>>
>>> +/* Return the mode for a vector that has NUNITS integer elements of
>>> +   INT_BITS bits each, if such a mode exists.  The mode can be either
>>> +   an integer mode or a vector mode.  */
>>> +
>>> +opt_machine_mode
>>> +mode_for_int_vector (unsigned int int_bits, unsigned int nunits)
>>
>> That's more vector_int_mode_for_size (...), no?  Similar to int_mode_for_size
>> or mode_for_size.
>>
>> Ok with those renamings.  I wonder if int_vector_mode_for_vector_mode
>> is necessary -- is calling vector_int_mode_for_size
>> (GET_MODE_UNIT_BITSIZE (mode),
>> GET_MODE_NUNITS (mode)) too cumbersome?
>
> IMO yes :-)  It's certainly longer than the equivalent int_mode_for_mode
> expansion.

I see.

Patch is ok as-is then.

Thanks,
Richard.

> Thanks,
> Richard