| Message ID | 878thuitk6.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Make more use of opt_mode | expand |
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
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) +{ + 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) +{ + 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); + 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