On Fri, 3 Mar 2017, Jakub Jelinek wrote:
> Hi!
>
> When working on PR79812 which was caused by bugs in x86 define_insn
> (used vec_select with parallel as last operand with incorrect number of
> operands), I wrote following sanity checking.
>
> The thing is that e.g. simplify-rtx.c already has such assertions:
> if (!VECTOR_MODE_P (mode))
> {
> gcc_assert (VECTOR_MODE_P (GET_MODE (trueop0)));
> gcc_assert (mode == GET_MODE_INNER (GET_MODE (trueop0)));
> gcc_assert (GET_CODE (trueop1) == PARALLEL);
> gcc_assert (XVECLEN (trueop1, 0) == 1);
> gcc_assert (CONST_INT_P (XVECEXP (trueop1, 0, 0)));
> ...
> else
> {
> gcc_assert (VECTOR_MODE_P (GET_MODE (trueop0)));
> gcc_assert (GET_MODE_INNER (mode)
> == GET_MODE_INNER (GET_MODE (trueop0)));
> gcc_assert (GET_CODE (trueop1) == PARALLEL);
>
> if (GET_CODE (trueop0) == CONST_VECTOR)
> {
> int elt_size = GET_MODE_UNIT_SIZE (mode);
> unsigned n_elts = (GET_MODE_SIZE (mode) / elt_size);
> rtvec v = rtvec_alloc (n_elts);
> unsigned int i;
>
> gcc_assert (XVECLEN (trueop1, 0) == (int) n_elts);
> ...
> and documentation says:
> @findex vec_select
> @item (vec_select:@var{m} @var{vec1} @var{selection})
> This describes an operation that selects parts of a vector. @var{vec1} is
> the source vector, and @var{selection} is a @code{parallel} that contains a
> @code{const_int} for each of the subparts of the result vector, giving the
> number of the source subpart that should be stored into it.
> The result mode @var{m} is either the submode for a single element of
> @var{vec1} (if only one subpart is selected), or another vector mode
> with that element submode (if multiple subparts are selected).
>
> Unfortunately, even after fixing 2 x86 bugs with that, I'm seeing tons
> of issues on other targets (many others are ok though) below. So, is the
> verification the right thing and are all these md bugs that should be fixed
> (and then the verification added)? If not, I'd be afraid if people are
> unlucky and combiner constant propagates something or some other pass,
> we can ICE on those in simplify-rtx.c. E.g. in vsx.md the thing is that
> the pattern uses an iterator with 2 V2?? modes in it and then V1TI mode,
> and uses exactly two elements in paralle, which doesn't make any sense
> for V1TI.
>
> ../../gcc/config/rs6000/vsx.md:2063:1: vec_select parallel with 2 elements,
> expected 1
> ../../gcc/config/rs6000/vsx.md:2112:1: vec_select parallel with 2 elements,
> expected 1
> ../../gcc/config/rs6000/vsx.md:2161:1: vec_select parallel with 2 elements,
> expected 1
>
> ../../gcc/config/aarch64/aarch64-simd.md:79:1: DImode of first vec_select
> operand is not a vector mode
> ../../gcc/config/aarch64/aarch64-simd.md:79:1: DFmode of first vec_select
> operand is not a vector mode
> ../../gcc/config/aarch64/aarch64-simd.md:588:1: DImode of first vec_select
> operand is not a vector mode
> ../../gcc/config/aarch64/aarch64-simd.md:588:1: DFmode of first vec_select
> operand is not a vector mode
> ../../gcc/config/aarch64/aarch64-simd.md:3192:1: DFmode of first vec_select
> operand is not a vector mode
>
> ../../gcc/config/arm/neon.md:1338:1: element mode mismatch between vec_select
> HImode and its operand QImode
> ../../gcc/config/arm/neon.md:1338:1: element mode mismatch between vec_select
> SImode and its operand QImode
> ../../gcc/config/arm/neon.md:1338:1: element mode mismatch between vec_select
> QImode and its operand HImode
> ../../gcc/config/arm/neon.md:1338:1: element mode mismatch between vec_select
> SImode and its operand HImode
> ../../gcc/config/arm/neon.md:1338:1: element mode mismatch between vec_select
> QImode and its operand SImode
> ../../gcc/config/arm/neon.md:1338:1: element mode mismatch between vec_select
> HImode and its operand SImode
> ../../gcc/config/arm/neon.md:1353:1: element mode mismatch between vec_select
> HImode and its operand QImode
> ../../gcc/config/arm/neon.md:1353:1: element mode mismatch between vec_select
> SImode and its operand QImode
> ../../gcc/config/arm/neon.md:1353:1: element mode mismatch between vec_select
> QImode and its operand HImode
> ../../gcc/config/arm/neon.md:1353:1: element mode mismatch between vec_select
> SImode and its operand HImode
> ../../gcc/config/arm/neon.md:1353:1: element mode mismatch between vec_select
> QImode and its operand SImode
> ../../gcc/config/arm/neon.md:1353:1: element mode mismatch between vec_select
> HImode and its operand SImode
> ../../gcc/config/arm/neon.md:1407:1: element mode mismatch between vec_select
> HImode and its operand QImode
> ../../gcc/config/arm/neon.md:1407:1: element mode mismatch between vec_select
> SImode and its operand QImode
> ../../gcc/config/arm/neon.md:1407:1: element mode mismatch between vec_select
> QImode and its operand HImode
> ../../gcc/config/arm/neon.md:1407:1: element mode mismatch between vec_select
> SImode and its operand HImode
> ../../gcc/config/arm/neon.md:1407:1: element mode mismatch between vec_select
> QImode and its operand SImode
> ../../gcc/config/arm/neon.md:1407:1: element mode mismatch between vec_select
> HImode and its operand SImode
> ../../gcc/config/arm/neon.md:1422:1: element mode mismatch between vec_select
> HImode and its operand QImode
> ../../gcc/config/arm/neon.md:1422:1: element mode mismatch between vec_select
> SImode and its operand QImode
> ../../gcc/config/arm/neon.md:1422:1: element mode mismatch between vec_select
> QImode and its operand HImode
> ../../gcc/config/arm/neon.md:1422:1: element mode mismatch between vec_select
> SImode and its operand HImode
> ../../gcc/config/arm/neon.md:1422:1: element mode mismatch between vec_select
> QImode and its operand SImode
> ../../gcc/config/arm/neon.md:1422:1: element mode mismatch between vec_select
> HImode and its operand SImode
>
> ../../gcc/config/mips/mips-msa.md:1219:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1219:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1219:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1219:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1307:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1307:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1307:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1307:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1401:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1401:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1401:1: vec_select parallel with 2
> elements, expected 4
> ../../gcc/config/mips/mips-msa.md:1401:1: vec_select parallel with 2
> elements, expected 4
I think these are all bugs and should be fixed and thus this checking
is good.
Of course we'd better not break (too many) targets at this point...
Richard.
> 2017-03-03 Jakub Jelinek <ja...@redhat.com>
>
> * genrecog.c (validate_pattern): Add VEC_SELECT validation.
> * genmodes.c (emit_min_insn_modes_c): Call emit_mode_nunits
> and emit_mode_inner.
>
> --- gcc/genrecog.c.jj 2017-01-01 12:45:35.000000000 +0100
> +++ gcc/genrecog.c 2017-03-03 17:07:23.591178911 +0100
> @@ -737,6 +737,32 @@ validate_pattern (rtx pattern, md_rtx_in
> GET_MODE_NAME (GET_MODE (XEXP (pattern, 0))));
> break;
>
> + case VEC_SELECT:
> + if (GET_MODE (pattern) != VOIDmode)
> + {
> + enum machine_mode mode = GET_MODE (pattern);
> + enum machine_mode imode = GET_MODE (XEXP (pattern, 0));
> + enum machine_mode emode
> + = VECTOR_MODE_P (mode) ? GET_MODE_INNER (mode) : mode;
> + if (GET_CODE (XEXP (pattern, 1)) == PARALLEL)
> + {
> + int expected = VECTOR_MODE_P (mode) ? GET_MODE_NUNITS (mode) : 1;
> + if (XVECLEN (XEXP (pattern, 1), 0) != expected)
> + error_at (info->loc,
> + "vec_select parallel with %d elements, expected %d",
> + XVECLEN (XEXP (pattern, 1), 0), expected);
> + }
> + if (imode != VOIDmode && !VECTOR_MODE_P (imode))
> + error_at (info->loc, "%smode of first vec_select operand is not a "
> + "vector mode", GET_MODE_NAME (imode));
> + else if (imode != VOIDmode && GET_MODE_INNER (imode) != emode)
> + error_at (info->loc, "element mode mismatch between vec_select "
> + "%smode and its operand %smode",
> + GET_MODE_NAME (emode),
> + GET_MODE_NAME (GET_MODE_INNER (imode)));
> + }
> + break;
> +
> default:
> break;
> }
> --- gcc/genmodes.c.jj 2017-01-01 12:45:39.000000000 +0100
> +++ gcc/genmodes.c 2017-03-03 17:06:43.009718721 +0100
> @@ -1789,7 +1789,9 @@ emit_min_insn_modes_c (void)
> emit_min_insn_modes_c_header ();
> emit_mode_name ();
> emit_mode_class ();
> + emit_mode_nunits ();
> emit_mode_wider ();
> + emit_mode_inner ();
> emit_class_narrowest_mode ();
> }
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
