This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] VEC_SELECT sanity checking in genrecog


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  <jakub@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]