[2/2] PR96463 -- changes to type checking vec_perm_expr in middle end

  Hi,
The attached patch rearranges order of type-check for vec_perm_expr
and relaxes type checking for
lhs = vec_perm_expr<rhs1, rhs2, mask>

when:
rhs1 == rhs2,
lhs is variable length vector,
rhs1 is fixed length vector,
TREE_TYPE (lhs) == TREE_TYPE (rhs1)

I am not sure tho if this check is correct ? My intent was to capture
case when vec_perm_expr is used to "extend" fixed length vector to
it's VLA equivalent.

Thanks,
Prathamesh
  

Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> Hi,
> The attached patch rearranges order of type-check for vec_perm_expr
> and relaxes type checking for
> lhs = vec_perm_expr<rhs1, rhs2, mask>
>
> when:
> rhs1 == rhs2,
> lhs is variable length vector,
> rhs1 is fixed length vector,
> TREE_TYPE (lhs) == TREE_TYPE (rhs1)
>
> I am not sure tho if this check is correct ? My intent was to capture
> case when vec_perm_expr is used to "extend" fixed length vector to
> it's VLA equivalent.

VLAness isn't really the issue.  We want the same thing to work for
-msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
vectors are fixed-length in that case.

The principle is that for:

  A = VEC_PERM_EXPR <B, C, D>;

the requirements are:

- A, B, C and D must be vectors
- A, B and C must have the same element type
- D must have an integer element type
- A and D must have the same number of elements (NA)
- B and C must have the same number of elements (NB)

The semantics are that we create a joined vector BC (all elements of B
followed by all element of C) and that:

  A[i] = BC[D[i] % (NB+NB)]

for 0 ≤ i < NA.

This operation makes sense even if NA != NB.

Thanks,
Richard

>
> Thanks,
> Prathamesh
>
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index 672e384ef09..9f91878c468 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -4325,10 +4325,11 @@ verify_gimple_assign_ternary (gassign *stmt)
>        break;
>  
>      case VEC_PERM_EXPR:
> -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> -	  || !useless_type_conversion_p (lhs_type, rhs2_type))
> +      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> +	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
> +	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
>  	{
> -	  error ("type mismatch in %qs", code_name);
> +	  error ("vector types expected in %qs", code_name);
>  	  debug_generic_expr (lhs_type);
>  	  debug_generic_expr (rhs1_type);
>  	  debug_generic_expr (rhs2_type);
> @@ -4336,11 +4337,14 @@ verify_gimple_assign_ternary (gassign *stmt)
>  	  return true;
>  	}
>  
> -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> -	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
> -	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> +      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
> +	  || (TREE_CODE (rhs3) != VECTOR_CST
> +	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
> +				    (TREE_TYPE (rhs3_type)))
> +		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> +				       (TREE_TYPE (rhs1_type))))))
>  	{
> -	  error ("vector types expected in %qs", code_name);
> +	  error ("invalid mask type in %qs", code_name);
>  	  debug_generic_expr (lhs_type);
>  	  debug_generic_expr (rhs1_type);
>  	  debug_generic_expr (rhs2_type);
> @@ -4348,15 +4352,18 @@ verify_gimple_assign_ternary (gassign *stmt)
>  	  return true;
>  	}
>  
> -      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> -		    TYPE_VECTOR_SUBPARTS (rhs2_type))
> -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> -		       TYPE_VECTOR_SUBPARTS (rhs3_type))
> -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> -		       TYPE_VECTOR_SUBPARTS (lhs_type)))
> +      /* Accept lhs = vec_perm_expr<v, v, mask> if lhs is vector length agnostic,
> +	 and has same element type as v.  */
> +      if (!TYPE_VECTOR_SUBPARTS (lhs_type).is_constant ()
> +	  && operand_equal_p (rhs1, rhs2, 0)
> +	  && TYPE_VECTOR_SUBPARTS (rhs1_type).is_constant ()
> +	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)) 
> +	return false;
> +
> +      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> +	  || !useless_type_conversion_p (lhs_type, rhs2_type))
>  	{
> -	  error ("vectors with different element number found in %qs",
> -		 code_name);
> +	  error ("type mismatch in %qs", code_name);
>  	  debug_generic_expr (lhs_type);
>  	  debug_generic_expr (rhs1_type);
>  	  debug_generic_expr (rhs2_type);
> @@ -4364,21 +4371,21 @@ verify_gimple_assign_ternary (gassign *stmt)
>  	  return true;
>  	}
>  
> -      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
> -	  || (TREE_CODE (rhs3) != VECTOR_CST
> -	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
> -				    (TREE_TYPE (rhs3_type)))
> -		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> -				       (TREE_TYPE (rhs1_type))))))
> +      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> +		    TYPE_VECTOR_SUBPARTS (rhs2_type))
> +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> +		       TYPE_VECTOR_SUBPARTS (rhs3_type))
> +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> +		       TYPE_VECTOR_SUBPARTS (lhs_type)))
>  	{
> -	  error ("invalid mask type in %qs", code_name);
> +	  error ("vectors with different element number found in %qs",
> +		 code_name);
>  	  debug_generic_expr (lhs_type);
>  	  debug_generic_expr (rhs1_type);
>  	  debug_generic_expr (rhs2_type);
>  	  debug_generic_expr (rhs3_type);
>  	  return true;
>  	}
> -
>        return false;
>  
>      case SAD_EXPR:
  

On Fri, 17 Dec 2021 at 16:37, Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > Hi,
> > The attached patch rearranges order of type-check for vec_perm_expr
> > and relaxes type checking for
> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> >
> > when:
> > rhs1 == rhs2,
> > lhs is variable length vector,
> > rhs1 is fixed length vector,
> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> >
> > I am not sure tho if this check is correct ? My intent was to capture
> > case when vec_perm_expr is used to "extend" fixed length vector to
> > it's VLA equivalent.
>
> VLAness isn't really the issue.  We want the same thing to work for
> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> vectors are fixed-length in that case.
>
> The principle is that for:
>
>   A = VEC_PERM_EXPR <B, C, D>;
>
> the requirements are:
>
> - A, B, C and D must be vectors
> - A, B and C must have the same element type
> - D must have an integer element type
> - A and D must have the same number of elements (NA)
> - B and C must have the same number of elements (NB)
>
> The semantics are that we create a joined vector BC (all elements of B
> followed by all element of C) and that:
>
>   A[i] = BC[D[i] % (NB+NB)]
>
> for 0 ≤ i < NA.
>
> This operation makes sense even if NA != NB.
Thanks for the suggestions, I tried to modify the patch accordingly.
Does it look OK ?
Passes bootstrap+test on aarch64-linux-gnu.

Thanks,
Prathamesh

>
> Thanks,
> Richard
>
> >
> > Thanks,
> > Prathamesh
> >
> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> > index 672e384ef09..9f91878c468 100644
> > --- a/gcc/tree-cfg.c
> > +++ b/gcc/tree-cfg.c
> > @@ -4325,10 +4325,11 @@ verify_gimple_assign_ternary (gassign *stmt)
> >        break;
> >
> >      case VEC_PERM_EXPR:
> > -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > -       || !useless_type_conversion_p (lhs_type, rhs2_type))
> > +      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > +       || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > +       || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> >       {
> > -       error ("type mismatch in %qs", code_name);
> > +       error ("vector types expected in %qs", code_name);
> >         debug_generic_expr (lhs_type);
> >         debug_generic_expr (rhs1_type);
> >         debug_generic_expr (rhs2_type);
> > @@ -4336,11 +4337,14 @@ verify_gimple_assign_ternary (gassign *stmt)
> >         return true;
> >       }
> >
> > -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > -       || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > -       || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> > +      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
> > +       || (TREE_CODE (rhs3) != VECTOR_CST
> > +           && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
> > +                                 (TREE_TYPE (rhs3_type)))
> > +               != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> > +                                    (TREE_TYPE (rhs1_type))))))
> >       {
> > -       error ("vector types expected in %qs", code_name);
> > +       error ("invalid mask type in %qs", code_name);
> >         debug_generic_expr (lhs_type);
> >         debug_generic_expr (rhs1_type);
> >         debug_generic_expr (rhs2_type);
> > @@ -4348,15 +4352,18 @@ verify_gimple_assign_ternary (gassign *stmt)
> >         return true;
> >       }
> >
> > -      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> > -                 TYPE_VECTOR_SUBPARTS (rhs2_type))
> > -       || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > -                    TYPE_VECTOR_SUBPARTS (rhs3_type))
> > -       || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> > -                    TYPE_VECTOR_SUBPARTS (lhs_type)))
> > +      /* Accept lhs = vec_perm_expr<v, v, mask> if lhs is vector length agnostic,
> > +      and has same element type as v.  */
> > +      if (!TYPE_VECTOR_SUBPARTS (lhs_type).is_constant ()
> > +       && operand_equal_p (rhs1, rhs2, 0)
> > +       && TYPE_VECTOR_SUBPARTS (rhs1_type).is_constant ()
> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type))
> > +     return false;
> > +
> > +      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > +       || !useless_type_conversion_p (lhs_type, rhs2_type))
> >       {
> > -       error ("vectors with different element number found in %qs",
> > -              code_name);
> > +       error ("type mismatch in %qs", code_name);
> >         debug_generic_expr (lhs_type);
> >         debug_generic_expr (rhs1_type);
> >         debug_generic_expr (rhs2_type);
> > @@ -4364,21 +4371,21 @@ verify_gimple_assign_ternary (gassign *stmt)
> >         return true;
> >       }
> >
> > -      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
> > -       || (TREE_CODE (rhs3) != VECTOR_CST
> > -           && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
> > -                                 (TREE_TYPE (rhs3_type)))
> > -               != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> > -                                    (TREE_TYPE (rhs1_type))))))
> > +      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> > +                 TYPE_VECTOR_SUBPARTS (rhs2_type))
> > +       || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > +                    TYPE_VECTOR_SUBPARTS (rhs3_type))
> > +       || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> > +                    TYPE_VECTOR_SUBPARTS (lhs_type)))
> >       {
> > -       error ("invalid mask type in %qs", code_name);
> > +       error ("vectors with different element number found in %qs",
> > +              code_name);
> >         debug_generic_expr (lhs_type);
> >         debug_generic_expr (rhs1_type);
> >         debug_generic_expr (rhs2_type);
> >         debug_generic_expr (rhs3_type);
> >         return true;
> >       }
> > -
> >        return false;
> >
> >      case SAD_EXPR:
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 672e384ef09..acc835c77cc 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -4325,10 +4325,12 @@ verify_gimple_assign_ternary (gassign *stmt)
       break;
 
     case VEC_PERM_EXPR:
-      if (!useless_type_conversion_p (lhs_type, rhs1_type)
-	  || !useless_type_conversion_p (lhs_type, rhs2_type))
+      if (TREE_CODE (lhs_type) != VECTOR_TYPE
+	  || TREE_CODE (rhs1_type) != VECTOR_TYPE
+	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
+	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
 	{
-	  error ("type mismatch in %qs", code_name);
+	  error ("vector types expected in %qs", code_name);
 	  debug_generic_expr (lhs_type);
 	  debug_generic_expr (rhs1_type);
 	  debug_generic_expr (rhs2_type);
@@ -4336,11 +4338,14 @@ verify_gimple_assign_ternary (gassign *stmt)
 	  return true;
 	}
 
-      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
-	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
-	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
+      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
+	  || (TREE_CODE (rhs3) != VECTOR_CST
+	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
+				    (TREE_TYPE (rhs3_type)))
+		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
+				       (TREE_TYPE (rhs1_type))))))
 	{
-	  error ("vector types expected in %qs", code_name);
+	  error ("invalid mask type in %qs", code_name);
 	  debug_generic_expr (lhs_type);
 	  debug_generic_expr (rhs1_type);
 	  debug_generic_expr (rhs2_type);
@@ -4348,15 +4353,23 @@ verify_gimple_assign_ternary (gassign *stmt)
 	  return true;
 	}
 
-      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
-		    TYPE_VECTOR_SUBPARTS (rhs2_type))
-	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
-		       TYPE_VECTOR_SUBPARTS (rhs3_type))
-	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
-		       TYPE_VECTOR_SUBPARTS (lhs_type)))
+      /* If lhs and rhs have different types, check that:
+         (a) Elements have same type.
+	 (b) lhs length == rhs3 length.
+	 (c) rhs1 length == rhs2 length.  */
+
+      if (!(types_compatible_p (lhs_type, rhs1_type)
+	    && types_compatible_p (lhs_type, rhs2_type))
+	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
+	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
+	  && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
+	  && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
+	return false;
+
+      if (!useless_type_conversion_p (lhs_type, rhs1_type)
+	  || !useless_type_conversion_p (lhs_type, rhs2_type))
 	{
-	  error ("vectors with different element number found in %qs",
-		 code_name);
+	  error ("type mismatch in %qs", code_name);
 	  debug_generic_expr (lhs_type);
 	  debug_generic_expr (rhs1_type);
 	  debug_generic_expr (rhs2_type);
@@ -4364,14 +4377,15 @@ verify_gimple_assign_ternary (gassign *stmt)
 	  return true;
 	}
 
-      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
-	  || (TREE_CODE (rhs3) != VECTOR_CST
-	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
-				    (TREE_TYPE (rhs3_type)))
-		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
-				       (TREE_TYPE (rhs1_type))))))
+      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
+		    TYPE_VECTOR_SUBPARTS (rhs2_type))
+	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
+		       TYPE_VECTOR_SUBPARTS (rhs3_type))
+	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
+		       TYPE_VECTOR_SUBPARTS (lhs_type)))
 	{
-	  error ("invalid mask type in %qs", code_name);
+	  error ("vectors with different element number found in %qs",
+		 code_name);
 	  debug_generic_expr (lhs_type);
 	  debug_generic_expr (rhs1_type);
 	  debug_generic_expr (rhs2_type);
  

On Fri, 17 Dec 2021, Richard Sandiford wrote:

> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > Hi,
> > The attached patch rearranges order of type-check for vec_perm_expr
> > and relaxes type checking for
> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> >
> > when:
> > rhs1 == rhs2,
> > lhs is variable length vector,
> > rhs1 is fixed length vector,
> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> >
> > I am not sure tho if this check is correct ? My intent was to capture
> > case when vec_perm_expr is used to "extend" fixed length vector to
> > it's VLA equivalent.
> 
> VLAness isn't really the issue.  We want the same thing to work for
> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> vectors are fixed-length in that case.
> 
> The principle is that for:
> 
>   A = VEC_PERM_EXPR <B, C, D>;
> 
> the requirements are:
> 
> - A, B, C and D must be vectors
> - A, B and C must have the same element type
> - D must have an integer element type
> - A and D must have the same number of elements (NA)
> - B and C must have the same number of elements (NB)
> 
> The semantics are that we create a joined vector BC (all elements of B
> followed by all element of C) and that:
> 
>   A[i] = BC[D[i] % (NB+NB)]
> 
> for 0 ≤ i < NA.
> 
> This operation makes sense even if NA != NB.

But note that we don't currently expect NA != NB and the optab just
has a single mode.

I'd rather go with the simpler patch I posted as reply to the earlier
mail rather such large refactoring at this point.

Richard.

> Thanks,
> Richard
> 
> >
> > Thanks,
> > Prathamesh
> >
> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> > index 672e384ef09..9f91878c468 100644
> > --- a/gcc/tree-cfg.c
> > +++ b/gcc/tree-cfg.c
> > @@ -4325,10 +4325,11 @@ verify_gimple_assign_ternary (gassign *stmt)
> >        break;
> >  
> >      case VEC_PERM_EXPR:
> > -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > -	  || !useless_type_conversion_p (lhs_type, rhs2_type))
> > +      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > +	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > +	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> >  	{
> > -	  error ("type mismatch in %qs", code_name);
> > +	  error ("vector types expected in %qs", code_name);
> >  	  debug_generic_expr (lhs_type);
> >  	  debug_generic_expr (rhs1_type);
> >  	  debug_generic_expr (rhs2_type);
> > @@ -4336,11 +4337,14 @@ verify_gimple_assign_ternary (gassign *stmt)
> >  	  return true;
> >  	}
> >  
> > -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > -	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > -	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> > +      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
> > +	  || (TREE_CODE (rhs3) != VECTOR_CST
> > +	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
> > +				    (TREE_TYPE (rhs3_type)))
> > +		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> > +				       (TREE_TYPE (rhs1_type))))))
> >  	{
> > -	  error ("vector types expected in %qs", code_name);
> > +	  error ("invalid mask type in %qs", code_name);
> >  	  debug_generic_expr (lhs_type);
> >  	  debug_generic_expr (rhs1_type);
> >  	  debug_generic_expr (rhs2_type);
> > @@ -4348,15 +4352,18 @@ verify_gimple_assign_ternary (gassign *stmt)
> >  	  return true;
> >  	}
> >  
> > -      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> > -		    TYPE_VECTOR_SUBPARTS (rhs2_type))
> > -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > -		       TYPE_VECTOR_SUBPARTS (rhs3_type))
> > -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> > -		       TYPE_VECTOR_SUBPARTS (lhs_type)))
> > +      /* Accept lhs = vec_perm_expr<v, v, mask> if lhs is vector length agnostic,
> > +	 and has same element type as v.  */
> > +      if (!TYPE_VECTOR_SUBPARTS (lhs_type).is_constant ()
> > +	  && operand_equal_p (rhs1, rhs2, 0)
> > +	  && TYPE_VECTOR_SUBPARTS (rhs1_type).is_constant ()
> > +	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)) 
> > +	return false;
> > +
> > +      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > +	  || !useless_type_conversion_p (lhs_type, rhs2_type))
> >  	{
> > -	  error ("vectors with different element number found in %qs",
> > -		 code_name);
> > +	  error ("type mismatch in %qs", code_name);
> >  	  debug_generic_expr (lhs_type);
> >  	  debug_generic_expr (rhs1_type);
> >  	  debug_generic_expr (rhs2_type);
> > @@ -4364,21 +4371,21 @@ verify_gimple_assign_ternary (gassign *stmt)
> >  	  return true;
> >  	}
> >  
> > -      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
> > -	  || (TREE_CODE (rhs3) != VECTOR_CST
> > -	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
> > -				    (TREE_TYPE (rhs3_type)))
> > -		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> > -				       (TREE_TYPE (rhs1_type))))))
> > +      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> > +		    TYPE_VECTOR_SUBPARTS (rhs2_type))
> > +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > +		       TYPE_VECTOR_SUBPARTS (rhs3_type))
> > +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> > +		       TYPE_VECTOR_SUBPARTS (lhs_type)))
> >  	{
> > -	  error ("invalid mask type in %qs", code_name);
> > +	  error ("vectors with different element number found in %qs",
> > +		 code_name);
> >  	  debug_generic_expr (lhs_type);
> >  	  debug_generic_expr (rhs1_type);
> >  	  debug_generic_expr (rhs2_type);
> >  	  debug_generic_expr (rhs3_type);
> >  	  return true;
> >  	}
> > -
> >        return false;
> >  
> >      case SAD_EXPR:
>
  

Richard Biener <rguenther@suse.de> writes:
> On Fri, 17 Dec 2021, Richard Sandiford wrote:
>
>> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
>> > Hi,
>> > The attached patch rearranges order of type-check for vec_perm_expr
>> > and relaxes type checking for
>> > lhs = vec_perm_expr<rhs1, rhs2, mask>
>> >
>> > when:
>> > rhs1 == rhs2,
>> > lhs is variable length vector,
>> > rhs1 is fixed length vector,
>> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
>> >
>> > I am not sure tho if this check is correct ? My intent was to capture
>> > case when vec_perm_expr is used to "extend" fixed length vector to
>> > it's VLA equivalent.
>> 
>> VLAness isn't really the issue.  We want the same thing to work for
>> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
>> vectors are fixed-length in that case.
>> 
>> The principle is that for:
>> 
>>   A = VEC_PERM_EXPR <B, C, D>;
>> 
>> the requirements are:
>> 
>> - A, B, C and D must be vectors
>> - A, B and C must have the same element type
>> - D must have an integer element type
>> - A and D must have the same number of elements (NA)
>> - B and C must have the same number of elements (NB)
>> 
>> The semantics are that we create a joined vector BC (all elements of B
>> followed by all element of C) and that:
>> 
>>   A[i] = BC[D[i] % (NB+NB)]
>> 
>> for 0 ≤ i < NA.
>> 
>> This operation makes sense even if NA != NB.
>
> But note that we don't currently expect NA != NB and the optab just
> has a single mode.

True, but we only need this for constant permutes.  They are already
special in that they allow the index elements to be wider than the data
elements.

Thanks,
Richard

>
> I'd rather go with the simpler patch I posted as reply to the earlier
> mail rather such large refactoring at this point.
>
> Richard.
>
>> Thanks,
>> Richard
>> 
>> >
>> > Thanks,
>> > Prathamesh
>> >
>> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
>> > index 672e384ef09..9f91878c468 100644
>> > --- a/gcc/tree-cfg.c
>> > +++ b/gcc/tree-cfg.c
>> > @@ -4325,10 +4325,11 @@ verify_gimple_assign_ternary (gassign *stmt)
>> >        break;
>> >  
>> >      case VEC_PERM_EXPR:
>> > -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
>> > -	  || !useless_type_conversion_p (lhs_type, rhs2_type))
>> > +      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
>> > +	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
>> > +	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
>> >  	{
>> > -	  error ("type mismatch in %qs", code_name);
>> > +	  error ("vector types expected in %qs", code_name);
>> >  	  debug_generic_expr (lhs_type);
>> >  	  debug_generic_expr (rhs1_type);
>> >  	  debug_generic_expr (rhs2_type);
>> > @@ -4336,11 +4337,14 @@ verify_gimple_assign_ternary (gassign *stmt)
>> >  	  return true;
>> >  	}
>> >  
>> > -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
>> > -	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
>> > -	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
>> > +      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
>> > +	  || (TREE_CODE (rhs3) != VECTOR_CST
>> > +	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
>> > +				    (TREE_TYPE (rhs3_type)))
>> > +		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
>> > +				       (TREE_TYPE (rhs1_type))))))
>> >  	{
>> > -	  error ("vector types expected in %qs", code_name);
>> > +	  error ("invalid mask type in %qs", code_name);
>> >  	  debug_generic_expr (lhs_type);
>> >  	  debug_generic_expr (rhs1_type);
>> >  	  debug_generic_expr (rhs2_type);
>> > @@ -4348,15 +4352,18 @@ verify_gimple_assign_ternary (gassign *stmt)
>> >  	  return true;
>> >  	}
>> >  
>> > -      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
>> > -		    TYPE_VECTOR_SUBPARTS (rhs2_type))
>> > -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
>> > -		       TYPE_VECTOR_SUBPARTS (rhs3_type))
>> > -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
>> > -		       TYPE_VECTOR_SUBPARTS (lhs_type)))
>> > +      /* Accept lhs = vec_perm_expr<v, v, mask> if lhs is vector length agnostic,
>> > +	 and has same element type as v.  */
>> > +      if (!TYPE_VECTOR_SUBPARTS (lhs_type).is_constant ()
>> > +	  && operand_equal_p (rhs1, rhs2, 0)
>> > +	  && TYPE_VECTOR_SUBPARTS (rhs1_type).is_constant ()
>> > +	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)) 
>> > +	return false;
>> > +
>> > +      if (!useless_type_conversion_p (lhs_type, rhs1_type)
>> > +	  || !useless_type_conversion_p (lhs_type, rhs2_type))
>> >  	{
>> > -	  error ("vectors with different element number found in %qs",
>> > -		 code_name);
>> > +	  error ("type mismatch in %qs", code_name);
>> >  	  debug_generic_expr (lhs_type);
>> >  	  debug_generic_expr (rhs1_type);
>> >  	  debug_generic_expr (rhs2_type);
>> > @@ -4364,21 +4371,21 @@ verify_gimple_assign_ternary (gassign *stmt)
>> >  	  return true;
>> >  	}
>> >  
>> > -      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
>> > -	  || (TREE_CODE (rhs3) != VECTOR_CST
>> > -	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
>> > -				    (TREE_TYPE (rhs3_type)))
>> > -		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
>> > -				       (TREE_TYPE (rhs1_type))))))
>> > +      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
>> > +		    TYPE_VECTOR_SUBPARTS (rhs2_type))
>> > +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
>> > +		       TYPE_VECTOR_SUBPARTS (rhs3_type))
>> > +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
>> > +		       TYPE_VECTOR_SUBPARTS (lhs_type)))
>> >  	{
>> > -	  error ("invalid mask type in %qs", code_name);
>> > +	  error ("vectors with different element number found in %qs",
>> > +		 code_name);
>> >  	  debug_generic_expr (lhs_type);
>> >  	  debug_generic_expr (rhs1_type);
>> >  	  debug_generic_expr (rhs2_type);
>> >  	  debug_generic_expr (rhs3_type);
>> >  	  return true;
>> >  	}
>> > -
>> >        return false;
>> >  
>> >      case SAD_EXPR:
>>
  

On Tue, 4 Jan 2022, Richard Sandiford wrote:

> Richard Biener <rguenther@suse.de> writes:
> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
> >
> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> >> > Hi,
> >> > The attached patch rearranges order of type-check for vec_perm_expr
> >> > and relaxes type checking for
> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> >> >
> >> > when:
> >> > rhs1 == rhs2,
> >> > lhs is variable length vector,
> >> > rhs1 is fixed length vector,
> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> >> >
> >> > I am not sure tho if this check is correct ? My intent was to capture
> >> > case when vec_perm_expr is used to "extend" fixed length vector to
> >> > it's VLA equivalent.
> >> 
> >> VLAness isn't really the issue.  We want the same thing to work for
> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> >> vectors are fixed-length in that case.
> >> 
> >> The principle is that for:
> >> 
> >>   A = VEC_PERM_EXPR <B, C, D>;
> >> 
> >> the requirements are:
> >> 
> >> - A, B, C and D must be vectors
> >> - A, B and C must have the same element type
> >> - D must have an integer element type
> >> - A and D must have the same number of elements (NA)
> >> - B and C must have the same number of elements (NB)
> >> 
> >> The semantics are that we create a joined vector BC (all elements of B
> >> followed by all element of C) and that:
> >> 
> >>   A[i] = BC[D[i] % (NB+NB)]
> >> 
> >> for 0 ≤ i < NA.
> >> 
> >> This operation makes sense even if NA != NB.
> >
> > But note that we don't currently expect NA != NB and the optab just
> > has a single mode.
> 
> True, but we only need this for constant permutes.  They are already
> special in that they allow the index elements to be wider than the data
> elements.

OK, then we should reflect this in the stmt verification and only relax
the constant permute vector case and also amend the
TARGET_VECTORIZE_VEC_PERM_CONST accordingly.

For non-constant permutes the docs say the mode of vec_perm is
the common mode of operands 1 and 2 whilst the mode of operand 0
is unspecified - even unconstrained by the docs.  I'm not sure
if vec_perm expansion is expected to eventually FAIL.  Updating the
docs of vec_perm would be appreciated as well.

As said I prefer to not mangle the existing stmt checking too much
at this stage so minimal adjustment is prefered there.

Thanks,
Richard.

> Thanks,
> Richard
> 
> >
> > I'd rather go with the simpler patch I posted as reply to the earlier
> > mail rather such large refactoring at this point.
> >
> > Richard.
> >
> >> Thanks,
> >> Richard
> >> 
> >> >
> >> > Thanks,
> >> > Prathamesh
> >> >
> >> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> >> > index 672e384ef09..9f91878c468 100644
> >> > --- a/gcc/tree-cfg.c
> >> > +++ b/gcc/tree-cfg.c
> >> > @@ -4325,10 +4325,11 @@ verify_gimple_assign_ternary (gassign *stmt)
> >> >        break;
> >> >  
> >> >      case VEC_PERM_EXPR:
> >> > -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> >> > -	  || !useless_type_conversion_p (lhs_type, rhs2_type))
> >> > +      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> >> > +	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
> >> > +	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> >> >  	{
> >> > -	  error ("type mismatch in %qs", code_name);
> >> > +	  error ("vector types expected in %qs", code_name);
> >> >  	  debug_generic_expr (lhs_type);
> >> >  	  debug_generic_expr (rhs1_type);
> >> >  	  debug_generic_expr (rhs2_type);
> >> > @@ -4336,11 +4337,14 @@ verify_gimple_assign_ternary (gassign *stmt)
> >> >  	  return true;
> >> >  	}
> >> >  
> >> > -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> >> > -	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
> >> > -	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> >> > +      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
> >> > +	  || (TREE_CODE (rhs3) != VECTOR_CST
> >> > +	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
> >> > +				    (TREE_TYPE (rhs3_type)))
> >> > +		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> >> > +				       (TREE_TYPE (rhs1_type))))))
> >> >  	{
> >> > -	  error ("vector types expected in %qs", code_name);
> >> > +	  error ("invalid mask type in %qs", code_name);
> >> >  	  debug_generic_expr (lhs_type);
> >> >  	  debug_generic_expr (rhs1_type);
> >> >  	  debug_generic_expr (rhs2_type);
> >> > @@ -4348,15 +4352,18 @@ verify_gimple_assign_ternary (gassign *stmt)
> >> >  	  return true;
> >> >  	}
> >> >  
> >> > -      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> >> > -		    TYPE_VECTOR_SUBPARTS (rhs2_type))
> >> > -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> >> > -		       TYPE_VECTOR_SUBPARTS (rhs3_type))
> >> > -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> >> > -		       TYPE_VECTOR_SUBPARTS (lhs_type)))
> >> > +      /* Accept lhs = vec_perm_expr<v, v, mask> if lhs is vector length agnostic,
> >> > +	 and has same element type as v.  */
> >> > +      if (!TYPE_VECTOR_SUBPARTS (lhs_type).is_constant ()
> >> > +	  && operand_equal_p (rhs1, rhs2, 0)
> >> > +	  && TYPE_VECTOR_SUBPARTS (rhs1_type).is_constant ()
> >> > +	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)) 
> >> > +	return false;
> >> > +
> >> > +      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> >> > +	  || !useless_type_conversion_p (lhs_type, rhs2_type))
> >> >  	{
> >> > -	  error ("vectors with different element number found in %qs",
> >> > -		 code_name);
> >> > +	  error ("type mismatch in %qs", code_name);
> >> >  	  debug_generic_expr (lhs_type);
> >> >  	  debug_generic_expr (rhs1_type);
> >> >  	  debug_generic_expr (rhs2_type);
> >> > @@ -4364,21 +4371,21 @@ verify_gimple_assign_ternary (gassign *stmt)
> >> >  	  return true;
> >> >  	}
> >> >  
> >> > -      if (TREE_CODE (TREE_TYPE (rhs3_type)) != INTEGER_TYPE
> >> > -	  || (TREE_CODE (rhs3) != VECTOR_CST
> >> > -	      && (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE
> >> > -				    (TREE_TYPE (rhs3_type)))
> >> > -		  != GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> >> > -				       (TREE_TYPE (rhs1_type))))))
> >> > +      if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> >> > +		    TYPE_VECTOR_SUBPARTS (rhs2_type))
> >> > +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> >> > +		       TYPE_VECTOR_SUBPARTS (rhs3_type))
> >> > +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> >> > +		       TYPE_VECTOR_SUBPARTS (lhs_type)))
> >> >  	{
> >> > -	  error ("invalid mask type in %qs", code_name);
> >> > +	  error ("vectors with different element number found in %qs",
> >> > +		 code_name);
> >> >  	  debug_generic_expr (lhs_type);
> >> >  	  debug_generic_expr (rhs1_type);
> >> >  	  debug_generic_expr (rhs2_type);
> >> >  	  debug_generic_expr (rhs3_type);
> >> >  	  return true;
> >> >  	}
> >> > -
> >> >        return false;
> >> >  
> >> >      case SAD_EXPR:
> >> 
>
  

Richard Biener <rguenther@suse.de> writes:
> On Tue, 4 Jan 2022, Richard Sandiford wrote:
>
>> Richard Biener <rguenther@suse.de> writes:
>> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
>> >
>> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
>> >> > Hi,
>> >> > The attached patch rearranges order of type-check for vec_perm_expr
>> >> > and relaxes type checking for
>> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
>> >> >
>> >> > when:
>> >> > rhs1 == rhs2,
>> >> > lhs is variable length vector,
>> >> > rhs1 is fixed length vector,
>> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
>> >> >
>> >> > I am not sure tho if this check is correct ? My intent was to capture
>> >> > case when vec_perm_expr is used to "extend" fixed length vector to
>> >> > it's VLA equivalent.
>> >> 
>> >> VLAness isn't really the issue.  We want the same thing to work for
>> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
>> >> vectors are fixed-length in that case.
>> >> 
>> >> The principle is that for:
>> >> 
>> >>   A = VEC_PERM_EXPR <B, C, D>;
>> >> 
>> >> the requirements are:
>> >> 
>> >> - A, B, C and D must be vectors
>> >> - A, B and C must have the same element type
>> >> - D must have an integer element type
>> >> - A and D must have the same number of elements (NA)
>> >> - B and C must have the same number of elements (NB)
>> >> 
>> >> The semantics are that we create a joined vector BC (all elements of B
>> >> followed by all element of C) and that:
>> >> 
>> >>   A[i] = BC[D[i] % (NB+NB)]
>> >> 
>> >> for 0 ≤ i < NA.
>> >> 
>> >> This operation makes sense even if NA != NB.
>> >
>> > But note that we don't currently expect NA != NB and the optab just
>> > has a single mode.
>> 
>> True, but we only need this for constant permutes.  They are already
>> special in that they allow the index elements to be wider than the data
>> elements.
>
> OK, then we should reflect this in the stmt verification and only relax
> the constant permute vector case and also amend the
> TARGET_VECTORIZE_VEC_PERM_CONST accordingly.

Sounds good.

> For non-constant permutes the docs say the mode of vec_perm is
> the common mode of operands 1 and 2 whilst the mode of operand 0
> is unspecified - even unconstrained by the docs.  I'm not sure
> if vec_perm expansion is expected to eventually FAIL.  Updating the
> docs of vec_perm would be appreciated as well.

Yeah, I guess de facto operand 0 has to be the same mode as operands
1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
or self-explanatory at the time. :-)

> As said I prefer to not mangle the existing stmt checking too much
> at this stage so minimal adjustment is prefered there.

The PR is only an enhancement request rather than a bug, so I think the
patch would need to wait for GCC 13 whatever happens.

Thanks,
Richard
  

On Tue, 4 Jan 2022 at 19:12, Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <rguenther@suse.de> writes:
> > On Tue, 4 Jan 2022, Richard Sandiford wrote:
> >
> >> Richard Biener <rguenther@suse.de> writes:
> >> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
> >> >
> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> >> >> > Hi,
> >> >> > The attached patch rearranges order of type-check for vec_perm_expr
> >> >> > and relaxes type checking for
> >> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> >> >> >
> >> >> > when:
> >> >> > rhs1 == rhs2,
> >> >> > lhs is variable length vector,
> >> >> > rhs1 is fixed length vector,
> >> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> >> >> >
> >> >> > I am not sure tho if this check is correct ? My intent was to capture
> >> >> > case when vec_perm_expr is used to "extend" fixed length vector to
> >> >> > it's VLA equivalent.
> >> >>
> >> >> VLAness isn't really the issue.  We want the same thing to work for
> >> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> >> >> vectors are fixed-length in that case.
> >> >>
> >> >> The principle is that for:
> >> >>
> >> >>   A = VEC_PERM_EXPR <B, C, D>;
> >> >>
> >> >> the requirements are:
> >> >>
> >> >> - A, B, C and D must be vectors
> >> >> - A, B and C must have the same element type
> >> >> - D must have an integer element type
> >> >> - A and D must have the same number of elements (NA)
> >> >> - B and C must have the same number of elements (NB)
> >> >>
> >> >> The semantics are that we create a joined vector BC (all elements of B
> >> >> followed by all element of C) and that:
> >> >>
> >> >>   A[i] = BC[D[i] % (NB+NB)]
> >> >>
> >> >> for 0 ≤ i < NA.
> >> >>
> >> >> This operation makes sense even if NA != NB.
> >> >
> >> > But note that we don't currently expect NA != NB and the optab just
> >> > has a single mode.
> >>
> >> True, but we only need this for constant permutes.  They are already
> >> special in that they allow the index elements to be wider than the data
> >> elements.
> >
> > OK, then we should reflect this in the stmt verification and only relax
> > the constant permute vector case and also amend the
> > TARGET_VECTORIZE_VEC_PERM_CONST accordingly.
>
> Sounds good.
>
> > For non-constant permutes the docs say the mode of vec_perm is
> > the common mode of operands 1 and 2 whilst the mode of operand 0
> > is unspecified - even unconstrained by the docs.  I'm not sure
> > if vec_perm expansion is expected to eventually FAIL.  Updating the
> > docs of vec_perm would be appreciated as well.
>
> Yeah, I guess de facto operand 0 has to be the same mode as operands
> 1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
> or self-explanatory at the time. :-)
>
> > As said I prefer to not mangle the existing stmt checking too much
> > at this stage so minimal adjustment is prefered there.
>
> The PR is only an enhancement request rather than a bug, so I think the
> patch would need to wait for GCC 13 whatever happens.
Hi,
In attached patch, the type checking is relaxed only if mask is constant.
Does this look OK ?

Thanks,
Prathamesh
>
> Thanks,
> Richard
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index e321d929fd0..02b88f67855 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -4307,6 +4307,24 @@ verify_gimple_assign_ternary (gassign *stmt)
       break;
 
     case VEC_PERM_EXPR:
+      /* If permute is constant, then we allow for lhs and rhs
+	 to have different vector types, provided:
+	 (1) lhs, rhs1, rhs2, and rhs3 have same element type.
+	 (2) rhs3 vector has integer element type.
+	 (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
+
+      if (TREE_CONSTANT (rhs3)
+	  && VECTOR_TYPE_P (lhs_type)
+	  && VECTOR_TYPE_P (rhs1_type)
+	  && VECTOR_TYPE_P (rhs2_type)
+	  && VECTOR_TYPE_P (rhs3_type)
+	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
+	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
+	  && INTEGRAL_TYPE_P (TREE_TYPE (rhs3_type))
+	  && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
+	  && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
+	return false;
+
       if (!useless_type_conversion_p (lhs_type, rhs1_type)
 	  || !useless_type_conversion_p (lhs_type, rhs2_type))
 	{
  

Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> On Tue, 4 Jan 2022 at 19:12, Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Richard Biener <rguenther@suse.de> writes:
>> > On Tue, 4 Jan 2022, Richard Sandiford wrote:
>> >
>> >> Richard Biener <rguenther@suse.de> writes:
>> >> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
>> >> >
>> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
>> >> >> > Hi,
>> >> >> > The attached patch rearranges order of type-check for vec_perm_expr
>> >> >> > and relaxes type checking for
>> >> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
>> >> >> >
>> >> >> > when:
>> >> >> > rhs1 == rhs2,
>> >> >> > lhs is variable length vector,
>> >> >> > rhs1 is fixed length vector,
>> >> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
>> >> >> >
>> >> >> > I am not sure tho if this check is correct ? My intent was to capture
>> >> >> > case when vec_perm_expr is used to "extend" fixed length vector to
>> >> >> > it's VLA equivalent.
>> >> >>
>> >> >> VLAness isn't really the issue.  We want the same thing to work for
>> >> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
>> >> >> vectors are fixed-length in that case.
>> >> >>
>> >> >> The principle is that for:
>> >> >>
>> >> >>   A = VEC_PERM_EXPR <B, C, D>;
>> >> >>
>> >> >> the requirements are:
>> >> >>
>> >> >> - A, B, C and D must be vectors
>> >> >> - A, B and C must have the same element type
>> >> >> - D must have an integer element type
>> >> >> - A and D must have the same number of elements (NA)
>> >> >> - B and C must have the same number of elements (NB)
>> >> >>
>> >> >> The semantics are that we create a joined vector BC (all elements of B
>> >> >> followed by all element of C) and that:
>> >> >>
>> >> >>   A[i] = BC[D[i] % (NB+NB)]
>> >> >>
>> >> >> for 0 ≤ i < NA.
>> >> >>
>> >> >> This operation makes sense even if NA != NB.
>> >> >
>> >> > But note that we don't currently expect NA != NB and the optab just
>> >> > has a single mode.
>> >>
>> >> True, but we only need this for constant permutes.  They are already
>> >> special in that they allow the index elements to be wider than the data
>> >> elements.
>> >
>> > OK, then we should reflect this in the stmt verification and only relax
>> > the constant permute vector case and also amend the
>> > TARGET_VECTORIZE_VEC_PERM_CONST accordingly.
>>
>> Sounds good.
>>
>> > For non-constant permutes the docs say the mode of vec_perm is
>> > the common mode of operands 1 and 2 whilst the mode of operand 0
>> > is unspecified - even unconstrained by the docs.  I'm not sure
>> > if vec_perm expansion is expected to eventually FAIL.  Updating the
>> > docs of vec_perm would be appreciated as well.
>>
>> Yeah, I guess de facto operand 0 has to be the same mode as operands
>> 1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
>> or self-explanatory at the time. :-)
>>
>> > As said I prefer to not mangle the existing stmt checking too much
>> > at this stage so minimal adjustment is prefered there.
>>
>> The PR is only an enhancement request rather than a bug, so I think the
>> patch would need to wait for GCC 13 whatever happens.
> Hi,
> In attached patch, the type checking is relaxed only if mask is constant.
> Does this look OK ?
>
> Thanks,
> Prathamesh
>>
>> Thanks,
>> Richard
>
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index e321d929fd0..02b88f67855 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -4307,6 +4307,24 @@ verify_gimple_assign_ternary (gassign *stmt)
>        break;
>  
>      case VEC_PERM_EXPR:
> +      /* If permute is constant, then we allow for lhs and rhs
> +	 to have different vector types, provided:
> +	 (1) lhs, rhs1, rhs2, and rhs3 have same element type.

This isn't a requirement for rhs3.

> +	 (2) rhs3 vector has integer element type.
> +	 (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> +
> +      if (TREE_CONSTANT (rhs3)
> +	  && VECTOR_TYPE_P (lhs_type)
> +	  && VECTOR_TYPE_P (rhs1_type)
> +	  && VECTOR_TYPE_P (rhs2_type)
> +	  && VECTOR_TYPE_P (rhs3_type)
> +	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
> +	  && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
> +	  && INTEGRAL_TYPE_P (TREE_TYPE (rhs3_type))
> +	  && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
> +	  && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
> +	return false;
> +

I think this should be integrated into the existing conditions
rather than done as an initial special case.

It might make sense to start with:

      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
	{

but expanded to test lhs_type too.  Then the other parts of the new test
should be distributed across the existing conditions.

The type tests should use useless_type_conversion_p rather than ==.

Thanks,
Richard



>        if (!useless_type_conversion_p (lhs_type, rhs1_type)
>  	  || !useless_type_conversion_p (lhs_type, rhs2_type))
>  	{
  

On Tue, 3 May 2022 at 18:25, Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > On Tue, 4 Jan 2022 at 19:12, Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> >>
> >> Richard Biener <rguenther@suse.de> writes:
> >> > On Tue, 4 Jan 2022, Richard Sandiford wrote:
> >> >
> >> >> Richard Biener <rguenther@suse.de> writes:
> >> >> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
> >> >> >
> >> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> >> >> >> > Hi,
> >> >> >> > The attached patch rearranges order of type-check for vec_perm_expr
> >> >> >> > and relaxes type checking for
> >> >> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> >> >> >> >
> >> >> >> > when:
> >> >> >> > rhs1 == rhs2,
> >> >> >> > lhs is variable length vector,
> >> >> >> > rhs1 is fixed length vector,
> >> >> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> >> >> >> >
> >> >> >> > I am not sure tho if this check is correct ? My intent was to capture
> >> >> >> > case when vec_perm_expr is used to "extend" fixed length vector to
> >> >> >> > it's VLA equivalent.
> >> >> >>
> >> >> >> VLAness isn't really the issue.  We want the same thing to work for
> >> >> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> >> >> >> vectors are fixed-length in that case.
> >> >> >>
> >> >> >> The principle is that for:
> >> >> >>
> >> >> >>   A = VEC_PERM_EXPR <B, C, D>;
> >> >> >>
> >> >> >> the requirements are:
> >> >> >>
> >> >> >> - A, B, C and D must be vectors
> >> >> >> - A, B and C must have the same element type
> >> >> >> - D must have an integer element type
> >> >> >> - A and D must have the same number of elements (NA)
> >> >> >> - B and C must have the same number of elements (NB)
> >> >> >>
> >> >> >> The semantics are that we create a joined vector BC (all elements of B
> >> >> >> followed by all element of C) and that:
> >> >> >>
> >> >> >>   A[i] = BC[D[i] % (NB+NB)]
> >> >> >>
> >> >> >> for 0 ≤ i < NA.
> >> >> >>
> >> >> >> This operation makes sense even if NA != NB.
> >> >> >
> >> >> > But note that we don't currently expect NA != NB and the optab just
> >> >> > has a single mode.
> >> >>
> >> >> True, but we only need this for constant permutes.  They are already
> >> >> special in that they allow the index elements to be wider than the data
> >> >> elements.
> >> >
> >> > OK, then we should reflect this in the stmt verification and only relax
> >> > the constant permute vector case and also amend the
> >> > TARGET_VECTORIZE_VEC_PERM_CONST accordingly.
> >>
> >> Sounds good.
> >>
> >> > For non-constant permutes the docs say the mode of vec_perm is
> >> > the common mode of operands 1 and 2 whilst the mode of operand 0
> >> > is unspecified - even unconstrained by the docs.  I'm not sure
> >> > if vec_perm expansion is expected to eventually FAIL.  Updating the
> >> > docs of vec_perm would be appreciated as well.
> >>
> >> Yeah, I guess de facto operand 0 has to be the same mode as operands
> >> 1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
> >> or self-explanatory at the time. :-)
> >>
> >> > As said I prefer to not mangle the existing stmt checking too much
> >> > at this stage so minimal adjustment is prefered there.
> >>
> >> The PR is only an enhancement request rather than a bug, so I think the
> >> patch would need to wait for GCC 13 whatever happens.
> > Hi,
> > In attached patch, the type checking is relaxed only if mask is constant.
> > Does this look OK ?
> >
> > Thanks,
> > Prathamesh
> >>
> >> Thanks,
> >> Richard
> >
> > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > index e321d929fd0..02b88f67855 100644
> > --- a/gcc/tree-cfg.cc
> > +++ b/gcc/tree-cfg.cc
> > @@ -4307,6 +4307,24 @@ verify_gimple_assign_ternary (gassign *stmt)
> >        break;
> >
> >      case VEC_PERM_EXPR:
> > +      /* If permute is constant, then we allow for lhs and rhs
> > +      to have different vector types, provided:
> > +      (1) lhs, rhs1, rhs2, and rhs3 have same element type.
>
> This isn't a requirement for rhs3.
>
> > +      (2) rhs3 vector has integer element type.
> > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> > +
> > +      if (TREE_CONSTANT (rhs3)
> > +       && VECTOR_TYPE_P (lhs_type)
> > +       && VECTOR_TYPE_P (rhs1_type)
> > +       && VECTOR_TYPE_P (rhs2_type)
> > +       && VECTOR_TYPE_P (rhs3_type)
> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
> > +       && INTEGRAL_TYPE_P (TREE_TYPE (rhs3_type))
> > +       && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
> > +       && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
> > +     return false;
> > +
>
> I think this should be integrated into the existing conditions
> rather than done as an initial special case.
>
> It might make sense to start with:
>
>       if (TREE_CODE (rhs1_type) != VECTOR_TYPE
>           || TREE_CODE (rhs2_type) != VECTOR_TYPE
>           || TREE_CODE (rhs3_type) != VECTOR_TYPE)
>         {
>
> but expanded to test lhs_type too.  Then the other parts of the new test
> should be distributed across the existing conditions.
>
> The type tests should use useless_type_conversion_p rather than ==.
Hi Richard,
Thanks for the suggestions. In the attached patch, I tried to
distribute the checks
across existing conditions, does it look OK ?
I am not sure how to write tests for the type checks tho, does
gimple-fe support vec_perm_expr ?
I did a quick testsuite run for vect.exp and the patch doesn't seem to
cause any unexpected failures.

Thanks,
Prathamesh
>
> Thanks,
> Richard
>
>
>
> >        if (!useless_type_conversion_p (lhs_type, rhs1_type)
> >         || !useless_type_conversion_p (lhs_type, rhs2_type))
> >       {
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index e321d929fd0..a845c7fff93 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -4307,18 +4307,14 @@ verify_gimple_assign_ternary (gassign *stmt)
       break;
 
     case VEC_PERM_EXPR:
-      if (!useless_type_conversion_p (lhs_type, rhs1_type)
-	  || !useless_type_conversion_p (lhs_type, rhs2_type))
-	{
-	  error ("type mismatch in %qs", code_name);
-	  debug_generic_expr (lhs_type);
-	  debug_generic_expr (rhs1_type);
-	  debug_generic_expr (rhs2_type);
-	  debug_generic_expr (rhs3_type);
-	  return true;
-	}
+      /* If permute is constant, then we allow for lhs and rhs
+	 to have different vector types, provided:
+	 (1) lhs, rhs1, rhs2 have same element type.
+	 (2) rhs3 vector has integer element type.
+	 (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
 
-      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
+      if (TREE_CODE (lhs_type) != VECTOR_TYPE
+	  || TREE_CODE (rhs1_type) != VECTOR_TYPE
 	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
 	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
 	{
@@ -4330,10 +4326,29 @@ verify_gimple_assign_ternary (gassign *stmt)
 	  return true;
 	}
 
+      /* If lhs, rhs1, and rhs2 are different vector types,
+	 then relax the check if rhs3 is constant and lhs, rhs1, and rhs2
+	 have same element types.  */
+      if ((!useless_type_conversion_p (lhs_type, rhs1_type)
+	   || !useless_type_conversion_p (lhs_type, rhs2_type))
+	  && (!TREE_CONSTANT (rhs3)
+	      || TREE_TYPE (lhs_type) != TREE_TYPE (rhs1_type)
+	      || TREE_TYPE (lhs_type) != TREE_TYPE (rhs2_type)))
+	{
+	    error ("type mismatch in %qs", code_name);
+	    debug_generic_expr (lhs_type);
+	    debug_generic_expr (rhs1_type);
+	    debug_generic_expr (rhs2_type);
+	    debug_generic_expr (rhs3_type);
+	    return true;
+	}
+
+      /* If rhs3 is constant, relax the check len(rhs2) == len(rhs3).  */
       if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
 		    TYPE_VECTOR_SUBPARTS (rhs2_type))
-	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
+	  || (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
 		       TYPE_VECTOR_SUBPARTS (rhs3_type))
+	      && !TREE_CONSTANT (rhs3))
 	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
 		       TYPE_VECTOR_SUBPARTS (lhs_type)))
 	{
  

Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> On Tue, 3 May 2022 at 18:25, Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
>> > On Tue, 4 Jan 2022 at 19:12, Richard Sandiford
>> > <richard.sandiford@arm.com> wrote:
>> >>
>> >> Richard Biener <rguenther@suse.de> writes:
>> >> > On Tue, 4 Jan 2022, Richard Sandiford wrote:
>> >> >
>> >> >> Richard Biener <rguenther@suse.de> writes:
>> >> >> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
>> >> >> >
>> >> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
>> >> >> >> > Hi,
>> >> >> >> > The attached patch rearranges order of type-check for vec_perm_expr
>> >> >> >> > and relaxes type checking for
>> >> >> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
>> >> >> >> >
>> >> >> >> > when:
>> >> >> >> > rhs1 == rhs2,
>> >> >> >> > lhs is variable length vector,
>> >> >> >> > rhs1 is fixed length vector,
>> >> >> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
>> >> >> >> >
>> >> >> >> > I am not sure tho if this check is correct ? My intent was to capture
>> >> >> >> > case when vec_perm_expr is used to "extend" fixed length vector to
>> >> >> >> > it's VLA equivalent.
>> >> >> >>
>> >> >> >> VLAness isn't really the issue.  We want the same thing to work for
>> >> >> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
>> >> >> >> vectors are fixed-length in that case.
>> >> >> >>
>> >> >> >> The principle is that for:
>> >> >> >>
>> >> >> >>   A = VEC_PERM_EXPR <B, C, D>;
>> >> >> >>
>> >> >> >> the requirements are:
>> >> >> >>
>> >> >> >> - A, B, C and D must be vectors
>> >> >> >> - A, B and C must have the same element type
>> >> >> >> - D must have an integer element type
>> >> >> >> - A and D must have the same number of elements (NA)
>> >> >> >> - B and C must have the same number of elements (NB)
>> >> >> >>
>> >> >> >> The semantics are that we create a joined vector BC (all elements of B
>> >> >> >> followed by all element of C) and that:
>> >> >> >>
>> >> >> >>   A[i] = BC[D[i] % (NB+NB)]
>> >> >> >>
>> >> >> >> for 0 ≤ i < NA.
>> >> >> >>
>> >> >> >> This operation makes sense even if NA != NB.
>> >> >> >
>> >> >> > But note that we don't currently expect NA != NB and the optab just
>> >> >> > has a single mode.
>> >> >>
>> >> >> True, but we only need this for constant permutes.  They are already
>> >> >> special in that they allow the index elements to be wider than the data
>> >> >> elements.
>> >> >
>> >> > OK, then we should reflect this in the stmt verification and only relax
>> >> > the constant permute vector case and also amend the
>> >> > TARGET_VECTORIZE_VEC_PERM_CONST accordingly.
>> >>
>> >> Sounds good.
>> >>
>> >> > For non-constant permutes the docs say the mode of vec_perm is
>> >> > the common mode of operands 1 and 2 whilst the mode of operand 0
>> >> > is unspecified - even unconstrained by the docs.  I'm not sure
>> >> > if vec_perm expansion is expected to eventually FAIL.  Updating the
>> >> > docs of vec_perm would be appreciated as well.
>> >>
>> >> Yeah, I guess de facto operand 0 has to be the same mode as operands
>> >> 1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
>> >> or self-explanatory at the time. :-)
>> >>
>> >> > As said I prefer to not mangle the existing stmt checking too much
>> >> > at this stage so minimal adjustment is prefered there.
>> >>
>> >> The PR is only an enhancement request rather than a bug, so I think the
>> >> patch would need to wait for GCC 13 whatever happens.
>> > Hi,
>> > In attached patch, the type checking is relaxed only if mask is constant.
>> > Does this look OK ?
>> >
>> > Thanks,
>> > Prathamesh
>> >>
>> >> Thanks,
>> >> Richard
>> >
>> > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
>> > index e321d929fd0..02b88f67855 100644
>> > --- a/gcc/tree-cfg.cc
>> > +++ b/gcc/tree-cfg.cc
>> > @@ -4307,6 +4307,24 @@ verify_gimple_assign_ternary (gassign *stmt)
>> >        break;
>> >
>> >      case VEC_PERM_EXPR:
>> > +      /* If permute is constant, then we allow for lhs and rhs
>> > +      to have different vector types, provided:
>> > +      (1) lhs, rhs1, rhs2, and rhs3 have same element type.
>>
>> This isn't a requirement for rhs3.
>>
>> > +      (2) rhs3 vector has integer element type.
>> > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
>> > +
>> > +      if (TREE_CONSTANT (rhs3)
>> > +       && VECTOR_TYPE_P (lhs_type)
>> > +       && VECTOR_TYPE_P (rhs1_type)
>> > +       && VECTOR_TYPE_P (rhs2_type)
>> > +       && VECTOR_TYPE_P (rhs3_type)
>> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
>> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
>> > +       && INTEGRAL_TYPE_P (TREE_TYPE (rhs3_type))
>> > +       && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
>> > +       && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
>> > +     return false;
>> > +
>>
>> I think this should be integrated into the existing conditions
>> rather than done as an initial special case.
>>
>> It might make sense to start with:
>>
>>       if (TREE_CODE (rhs1_type) != VECTOR_TYPE
>>           || TREE_CODE (rhs2_type) != VECTOR_TYPE
>>           || TREE_CODE (rhs3_type) != VECTOR_TYPE)
>>         {
>>
>> but expanded to test lhs_type too.  Then the other parts of the new test
>> should be distributed across the existing conditions.
>>
>> The type tests should use useless_type_conversion_p rather than ==.
> Hi Richard,
> Thanks for the suggestions. In the attached patch, I tried to
> distribute the checks
> across existing conditions, does it look OK ?
> I am not sure how to write tests for the type checks tho, does
> gimple-fe support vec_perm_expr ?
> I did a quick testsuite run for vect.exp and the patch doesn't seem to
> cause any unexpected failures.
>
> Thanks,
> Prathamesh
>>
>> Thanks,
>> Richard
>>
>>
>>
>> >        if (!useless_type_conversion_p (lhs_type, rhs1_type)
>> >         || !useless_type_conversion_p (lhs_type, rhs2_type))
>> >       {
>
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index e321d929fd0..a845c7fff93 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -4307,18 +4307,14 @@ verify_gimple_assign_ternary (gassign *stmt)
>        break;
>  
>      case VEC_PERM_EXPR:
> -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> -	  || !useless_type_conversion_p (lhs_type, rhs2_type))
> -	{
> -	  error ("type mismatch in %qs", code_name);
> -	  debug_generic_expr (lhs_type);
> -	  debug_generic_expr (rhs1_type);
> -	  debug_generic_expr (rhs2_type);
> -	  debug_generic_expr (rhs3_type);
> -	  return true;
> -	}
> +      /* If permute is constant, then we allow for lhs and rhs
> +	 to have different vector types, provided:
> +	 (1) lhs, rhs1, rhs2 have same element type.
> +	 (2) rhs3 vector has integer element type.
> +	 (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
>  
> -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> +      if (TREE_CODE (lhs_type) != VECTOR_TYPE
> +	  || TREE_CODE (rhs1_type) != VECTOR_TYPE
>  	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
>  	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
>  	{
> @@ -4330,10 +4326,29 @@ verify_gimple_assign_ternary (gassign *stmt)
>  	  return true;
>  	}
>  
> +      /* If lhs, rhs1, and rhs2 are different vector types,
> +	 then relax the check if rhs3 is constant and lhs, rhs1, and rhs2
> +	 have same element types.  */
> +      if ((!useless_type_conversion_p (lhs_type, rhs1_type)
> +	   || !useless_type_conversion_p (lhs_type, rhs2_type))
> +	  && (!TREE_CONSTANT (rhs3)
> +	      || TREE_TYPE (lhs_type) != TREE_TYPE (rhs1_type)
> +	      || TREE_TYPE (lhs_type) != TREE_TYPE (rhs2_type)))

These TREE_TYPE tests should use !useless_type_conversion_p too,
instead of !=.  Maybe it would be easier to follow as:

  if (TREE_CONSTANT (rhs3)
      ? ...
      : ...)

so that we don't have doubled useless_type_conversion_p checks
for the TREE_CONSTANT case.

> +	{
> +	    error ("type mismatch in %qs", code_name);
> +	    debug_generic_expr (lhs_type);
> +	    debug_generic_expr (rhs1_type);
> +	    debug_generic_expr (rhs2_type);
> +	    debug_generic_expr (rhs3_type);
> +	    return true;
> +	}
> +
> +      /* If rhs3 is constant, relax the check len(rhs2) == len(rhs3).  */
>        if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
>  		    TYPE_VECTOR_SUBPARTS (rhs2_type))
> -	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> +	  || (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
>  		       TYPE_VECTOR_SUBPARTS (rhs3_type))
> +	      && !TREE_CONSTANT (rhs3))

Very minor, but I think this reads better with the !TREE_CONSTANT first
in the && rather than second.  There's no reason to compare the lengths
for TREE_CONSTANT.

Otherwise it looks good to me, but Richard should have the final say.

Thanks,
Richard

>  	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
>  		       TYPE_VECTOR_SUBPARTS (lhs_type)))
>  	{
  

On Mon, 9 May 2022 at 19:22, Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > On Tue, 3 May 2022 at 18:25, Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> >>
> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> >> > On Tue, 4 Jan 2022 at 19:12, Richard Sandiford
> >> > <richard.sandiford@arm.com> wrote:
> >> >>
> >> >> Richard Biener <rguenther@suse.de> writes:
> >> >> > On Tue, 4 Jan 2022, Richard Sandiford wrote:
> >> >> >
> >> >> >> Richard Biener <rguenther@suse.de> writes:
> >> >> >> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
> >> >> >> >
> >> >> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> >> >> >> >> > Hi,
> >> >> >> >> > The attached patch rearranges order of type-check for vec_perm_expr
> >> >> >> >> > and relaxes type checking for
> >> >> >> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> >> >> >> >> >
> >> >> >> >> > when:
> >> >> >> >> > rhs1 == rhs2,
> >> >> >> >> > lhs is variable length vector,
> >> >> >> >> > rhs1 is fixed length vector,
> >> >> >> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> >> >> >> >> >
> >> >> >> >> > I am not sure tho if this check is correct ? My intent was to capture
> >> >> >> >> > case when vec_perm_expr is used to "extend" fixed length vector to
> >> >> >> >> > it's VLA equivalent.
> >> >> >> >>
> >> >> >> >> VLAness isn't really the issue.  We want the same thing to work for
> >> >> >> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> >> >> >> >> vectors are fixed-length in that case.
> >> >> >> >>
> >> >> >> >> The principle is that for:
> >> >> >> >>
> >> >> >> >>   A = VEC_PERM_EXPR <B, C, D>;
> >> >> >> >>
> >> >> >> >> the requirements are:
> >> >> >> >>
> >> >> >> >> - A, B, C and D must be vectors
> >> >> >> >> - A, B and C must have the same element type
> >> >> >> >> - D must have an integer element type
> >> >> >> >> - A and D must have the same number of elements (NA)
> >> >> >> >> - B and C must have the same number of elements (NB)
> >> >> >> >>
> >> >> >> >> The semantics are that we create a joined vector BC (all elements of B
> >> >> >> >> followed by all element of C) and that:
> >> >> >> >>
> >> >> >> >>   A[i] = BC[D[i] % (NB+NB)]
> >> >> >> >>
> >> >> >> >> for 0 ≤ i < NA.
> >> >> >> >>
> >> >> >> >> This operation makes sense even if NA != NB.
> >> >> >> >
> >> >> >> > But note that we don't currently expect NA != NB and the optab just
> >> >> >> > has a single mode.
> >> >> >>
> >> >> >> True, but we only need this for constant permutes.  They are already
> >> >> >> special in that they allow the index elements to be wider than the data
> >> >> >> elements.
> >> >> >
> >> >> > OK, then we should reflect this in the stmt verification and only relax
> >> >> > the constant permute vector case and also amend the
> >> >> > TARGET_VECTORIZE_VEC_PERM_CONST accordingly.
> >> >>
> >> >> Sounds good.
> >> >>
> >> >> > For non-constant permutes the docs say the mode of vec_perm is
> >> >> > the common mode of operands 1 and 2 whilst the mode of operand 0
> >> >> > is unspecified - even unconstrained by the docs.  I'm not sure
> >> >> > if vec_perm expansion is expected to eventually FAIL.  Updating the
> >> >> > docs of vec_perm would be appreciated as well.
> >> >>
> >> >> Yeah, I guess de facto operand 0 has to be the same mode as operands
> >> >> 1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
> >> >> or self-explanatory at the time. :-)
> >> >>
> >> >> > As said I prefer to not mangle the existing stmt checking too much
> >> >> > at this stage so minimal adjustment is prefered there.
> >> >>
> >> >> The PR is only an enhancement request rather than a bug, so I think the
> >> >> patch would need to wait for GCC 13 whatever happens.
> >> > Hi,
> >> > In attached patch, the type checking is relaxed only if mask is constant.
> >> > Does this look OK ?
> >> >
> >> > Thanks,
> >> > Prathamesh
> >> >>
> >> >> Thanks,
> >> >> Richard
> >> >
> >> > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> >> > index e321d929fd0..02b88f67855 100644
> >> > --- a/gcc/tree-cfg.cc
> >> > +++ b/gcc/tree-cfg.cc
> >> > @@ -4307,6 +4307,24 @@ verify_gimple_assign_ternary (gassign *stmt)
> >> >        break;
> >> >
> >> >      case VEC_PERM_EXPR:
> >> > +      /* If permute is constant, then we allow for lhs and rhs
> >> > +      to have different vector types, provided:
> >> > +      (1) lhs, rhs1, rhs2, and rhs3 have same element type.
> >>
> >> This isn't a requirement for rhs3.
> >>
> >> > +      (2) rhs3 vector has integer element type.
> >> > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> >> > +
> >> > +      if (TREE_CONSTANT (rhs3)
> >> > +       && VECTOR_TYPE_P (lhs_type)
> >> > +       && VECTOR_TYPE_P (rhs1_type)
> >> > +       && VECTOR_TYPE_P (rhs2_type)
> >> > +       && VECTOR_TYPE_P (rhs3_type)
> >> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
> >> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
> >> > +       && INTEGRAL_TYPE_P (TREE_TYPE (rhs3_type))
> >> > +       && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
> >> > +       && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
> >> > +     return false;
> >> > +
> >>
> >> I think this should be integrated into the existing conditions
> >> rather than done as an initial special case.
> >>
> >> It might make sense to start with:
> >>
> >>       if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> >>           || TREE_CODE (rhs2_type) != VECTOR_TYPE
> >>           || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> >>         {
> >>
> >> but expanded to test lhs_type too.  Then the other parts of the new test
> >> should be distributed across the existing conditions.
> >>
> >> The type tests should use useless_type_conversion_p rather than ==.
> > Hi Richard,
> > Thanks for the suggestions. In the attached patch, I tried to
> > distribute the checks
> > across existing conditions, does it look OK ?
> > I am not sure how to write tests for the type checks tho, does
> > gimple-fe support vec_perm_expr ?
> > I did a quick testsuite run for vect.exp and the patch doesn't seem to
> > cause any unexpected failures.
> >
> > Thanks,
> > Prathamesh
> >>
> >> Thanks,
> >> Richard
> >>
> >>
> >>
> >> >        if (!useless_type_conversion_p (lhs_type, rhs1_type)
> >> >         || !useless_type_conversion_p (lhs_type, rhs2_type))
> >> >       {
> >
> > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > index e321d929fd0..a845c7fff93 100644
> > --- a/gcc/tree-cfg.cc
> > +++ b/gcc/tree-cfg.cc
> > @@ -4307,18 +4307,14 @@ verify_gimple_assign_ternary (gassign *stmt)
> >        break;
> >
> >      case VEC_PERM_EXPR:
> > -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > -       || !useless_type_conversion_p (lhs_type, rhs2_type))
> > -     {
> > -       error ("type mismatch in %qs", code_name);
> > -       debug_generic_expr (lhs_type);
> > -       debug_generic_expr (rhs1_type);
> > -       debug_generic_expr (rhs2_type);
> > -       debug_generic_expr (rhs3_type);
> > -       return true;
> > -     }
> > +      /* If permute is constant, then we allow for lhs and rhs
> > +      to have different vector types, provided:
> > +      (1) lhs, rhs1, rhs2 have same element type.
> > +      (2) rhs3 vector has integer element type.
> > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> >
> > -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > +      if (TREE_CODE (lhs_type) != VECTOR_TYPE
> > +       || TREE_CODE (rhs1_type) != VECTOR_TYPE
> >         || TREE_CODE (rhs2_type) != VECTOR_TYPE
> >         || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> >       {
> > @@ -4330,10 +4326,29 @@ verify_gimple_assign_ternary (gassign *stmt)
> >         return true;
> >       }
> >
> > +      /* If lhs, rhs1, and rhs2 are different vector types,
> > +      then relax the check if rhs3 is constant and lhs, rhs1, and rhs2
> > +      have same element types.  */
> > +      if ((!useless_type_conversion_p (lhs_type, rhs1_type)
> > +        || !useless_type_conversion_p (lhs_type, rhs2_type))
> > +       && (!TREE_CONSTANT (rhs3)
> > +           || TREE_TYPE (lhs_type) != TREE_TYPE (rhs1_type)
> > +           || TREE_TYPE (lhs_type) != TREE_TYPE (rhs2_type)))
>
> These TREE_TYPE tests should use !useless_type_conversion_p too,
> instead of !=.  Maybe it would be easier to follow as:
>
>   if (TREE_CONSTANT (rhs3)
>       ? ...
>       : ...)
>
> so that we don't have doubled useless_type_conversion_p checks
> for the TREE_CONSTANT case.
>
> > +     {
> > +         error ("type mismatch in %qs", code_name);
> > +         debug_generic_expr (lhs_type);
> > +         debug_generic_expr (rhs1_type);
> > +         debug_generic_expr (rhs2_type);
> > +         debug_generic_expr (rhs3_type);
> > +         return true;
> > +     }
> > +
> > +      /* If rhs3 is constant, relax the check len(rhs2) == len(rhs3).  */
> >        if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> >                   TYPE_VECTOR_SUBPARTS (rhs2_type))
> > -       || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > +       || (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> >                      TYPE_VECTOR_SUBPARTS (rhs3_type))
> > +           && !TREE_CONSTANT (rhs3))
>
> Very minor, but I think this reads better with the !TREE_CONSTANT first
> in the && rather than second.  There's no reason to compare the lengths
> for TREE_CONSTANT.
>
> Otherwise it looks good to me, but Richard should have the final say.
Thanks, I addressed the above suggestions in the attached patch.
Does it look OK ?

Thanks,
Prathamesh
>
> Thanks,
> Richard
>
> >         || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> >                      TYPE_VECTOR_SUBPARTS (lhs_type)))
> >       {
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index e321d929fd0..31f2514a407 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -4307,18 +4307,14 @@ verify_gimple_assign_ternary (gassign *stmt)
       break;
 
     case VEC_PERM_EXPR:
-      if (!useless_type_conversion_p (lhs_type, rhs1_type)
-	  || !useless_type_conversion_p (lhs_type, rhs2_type))
-	{
-	  error ("type mismatch in %qs", code_name);
-	  debug_generic_expr (lhs_type);
-	  debug_generic_expr (rhs1_type);
-	  debug_generic_expr (rhs2_type);
-	  debug_generic_expr (rhs3_type);
-	  return true;
-	}
+      /* If permute is constant, then we allow for lhs and rhs
+	 to have different vector types, provided:
+	 (1) lhs, rhs1, rhs2 have same element type.
+	 (2) rhs3 vector has integer element type.
+	 (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
 
-      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
+      if (TREE_CODE (lhs_type) != VECTOR_TYPE
+	  || TREE_CODE (rhs1_type) != VECTOR_TYPE
 	  || TREE_CODE (rhs2_type) != VECTOR_TYPE
 	  || TREE_CODE (rhs3_type) != VECTOR_TYPE)
 	{
@@ -4330,10 +4326,28 @@ verify_gimple_assign_ternary (gassign *stmt)
 	  return true;
 	}
 
+      /* If rhs3 is constant, we allow lhs, rhs1 and rhs2 to be different vector types,
+	 as long as lhs, rhs1 and rhs2 have same element type.  */
+      if (TREE_CONSTANT (rhs3)
+	  ? (!useless_type_conversion_p (TREE_TYPE (lhs_type), TREE_TYPE (rhs1_type))
+	     || !useless_type_conversion_p (TREE_TYPE (lhs_type), TREE_TYPE (rhs2_type)))
+	  : (!useless_type_conversion_p (lhs_type, rhs1_type)
+	     || !useless_type_conversion_p (lhs_type, rhs2_type)))
+	{
+	    error ("type mismatch in %qs", code_name);
+	    debug_generic_expr (lhs_type);
+	    debug_generic_expr (rhs1_type);
+	    debug_generic_expr (rhs2_type);
+	    debug_generic_expr (rhs3_type);
+	    return true;
+	}
+
+      /* If rhs3 is constant, relax the check len(rhs2) == len(rhs3).  */
       if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
 		    TYPE_VECTOR_SUBPARTS (rhs2_type))
-	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
-		       TYPE_VECTOR_SUBPARTS (rhs3_type))
+	  || (!TREE_CONSTANT(rhs3)
+	      && maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
+			   TYPE_VECTOR_SUBPARTS (rhs3_type)))
 	  || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
 		       TYPE_VECTOR_SUBPARTS (lhs_type)))
 	{
  

On Mon, 9 May 2022 at 21:21, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
>
> On Mon, 9 May 2022 at 19:22, Richard Sandiford
> <richard.sandiford@arm.com> wrote:
> >
> > Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > > On Tue, 3 May 2022 at 18:25, Richard Sandiford
> > > <richard.sandiford@arm.com> wrote:
> > >>
> > >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > >> > On Tue, 4 Jan 2022 at 19:12, Richard Sandiford
> > >> > <richard.sandiford@arm.com> wrote:
> > >> >>
> > >> >> Richard Biener <rguenther@suse.de> writes:
> > >> >> > On Tue, 4 Jan 2022, Richard Sandiford wrote:
> > >> >> >
> > >> >> >> Richard Biener <rguenther@suse.de> writes:
> > >> >> >> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
> > >> >> >> >
> > >> >> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > >> >> >> >> > Hi,
> > >> >> >> >> > The attached patch rearranges order of type-check for vec_perm_expr
> > >> >> >> >> > and relaxes type checking for
> > >> >> >> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> > >> >> >> >> >
> > >> >> >> >> > when:
> > >> >> >> >> > rhs1 == rhs2,
> > >> >> >> >> > lhs is variable length vector,
> > >> >> >> >> > rhs1 is fixed length vector,
> > >> >> >> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> > >> >> >> >> >
> > >> >> >> >> > I am not sure tho if this check is correct ? My intent was to capture
> > >> >> >> >> > case when vec_perm_expr is used to "extend" fixed length vector to
> > >> >> >> >> > it's VLA equivalent.
> > >> >> >> >>
> > >> >> >> >> VLAness isn't really the issue.  We want the same thing to work for
> > >> >> >> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> > >> >> >> >> vectors are fixed-length in that case.
> > >> >> >> >>
> > >> >> >> >> The principle is that for:
> > >> >> >> >>
> > >> >> >> >>   A = VEC_PERM_EXPR <B, C, D>;
> > >> >> >> >>
> > >> >> >> >> the requirements are:
> > >> >> >> >>
> > >> >> >> >> - A, B, C and D must be vectors
> > >> >> >> >> - A, B and C must have the same element type
> > >> >> >> >> - D must have an integer element type
> > >> >> >> >> - A and D must have the same number of elements (NA)
> > >> >> >> >> - B and C must have the same number of elements (NB)
> > >> >> >> >>
> > >> >> >> >> The semantics are that we create a joined vector BC (all elements of B
> > >> >> >> >> followed by all element of C) and that:
> > >> >> >> >>
> > >> >> >> >>   A[i] = BC[D[i] % (NB+NB)]
> > >> >> >> >>
> > >> >> >> >> for 0 ≤ i < NA.
> > >> >> >> >>
> > >> >> >> >> This operation makes sense even if NA != NB.
> > >> >> >> >
> > >> >> >> > But note that we don't currently expect NA != NB and the optab just
> > >> >> >> > has a single mode.
> > >> >> >>
> > >> >> >> True, but we only need this for constant permutes.  They are already
> > >> >> >> special in that they allow the index elements to be wider than the data
> > >> >> >> elements.
> > >> >> >
> > >> >> > OK, then we should reflect this in the stmt verification and only relax
> > >> >> > the constant permute vector case and also amend the
> > >> >> > TARGET_VECTORIZE_VEC_PERM_CONST accordingly.
> > >> >>
> > >> >> Sounds good.
> > >> >>
> > >> >> > For non-constant permutes the docs say the mode of vec_perm is
> > >> >> > the common mode of operands 1 and 2 whilst the mode of operand 0
> > >> >> > is unspecified - even unconstrained by the docs.  I'm not sure
> > >> >> > if vec_perm expansion is expected to eventually FAIL.  Updating the
> > >> >> > docs of vec_perm would be appreciated as well.
> > >> >>
> > >> >> Yeah, I guess de facto operand 0 has to be the same mode as operands
> > >> >> 1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
> > >> >> or self-explanatory at the time. :-)
> > >> >>
> > >> >> > As said I prefer to not mangle the existing stmt checking too much
> > >> >> > at this stage so minimal adjustment is prefered there.
> > >> >>
> > >> >> The PR is only an enhancement request rather than a bug, so I think the
> > >> >> patch would need to wait for GCC 13 whatever happens.
> > >> > Hi,
> > >> > In attached patch, the type checking is relaxed only if mask is constant.
> > >> > Does this look OK ?
> > >> >
> > >> > Thanks,
> > >> > Prathamesh
> > >> >>
> > >> >> Thanks,
> > >> >> Richard
> > >> >
> > >> > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > >> > index e321d929fd0..02b88f67855 100644
> > >> > --- a/gcc/tree-cfg.cc
> > >> > +++ b/gcc/tree-cfg.cc
> > >> > @@ -4307,6 +4307,24 @@ verify_gimple_assign_ternary (gassign *stmt)
> > >> >        break;
> > >> >
> > >> >      case VEC_PERM_EXPR:
> > >> > +      /* If permute is constant, then we allow for lhs and rhs
> > >> > +      to have different vector types, provided:
> > >> > +      (1) lhs, rhs1, rhs2, and rhs3 have same element type.
> > >>
> > >> This isn't a requirement for rhs3.
> > >>
> > >> > +      (2) rhs3 vector has integer element type.
> > >> > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> > >> > +
> > >> > +      if (TREE_CONSTANT (rhs3)
> > >> > +       && VECTOR_TYPE_P (lhs_type)
> > >> > +       && VECTOR_TYPE_P (rhs1_type)
> > >> > +       && VECTOR_TYPE_P (rhs2_type)
> > >> > +       && VECTOR_TYPE_P (rhs3_type)
> > >> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
> > >> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
> > >> > +       && INTEGRAL_TYPE_P (TREE_TYPE (rhs3_type))
> > >> > +       && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
> > >> > +       && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
> > >> > +     return false;
> > >> > +
> > >>
> > >> I think this should be integrated into the existing conditions
> > >> rather than done as an initial special case.
> > >>
> > >> It might make sense to start with:
> > >>
> > >>       if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > >>           || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > >>           || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> > >>         {
> > >>
> > >> but expanded to test lhs_type too.  Then the other parts of the new test
> > >> should be distributed across the existing conditions.
> > >>
> > >> The type tests should use useless_type_conversion_p rather than ==.
> > > Hi Richard,
> > > Thanks for the suggestions. In the attached patch, I tried to
> > > distribute the checks
> > > across existing conditions, does it look OK ?
> > > I am not sure how to write tests for the type checks tho, does
> > > gimple-fe support vec_perm_expr ?
> > > I did a quick testsuite run for vect.exp and the patch doesn't seem to
> > > cause any unexpected failures.
> > >
> > > Thanks,
> > > Prathamesh
> > >>
> > >> Thanks,
> > >> Richard
> > >>
> > >>
> > >>
> > >> >        if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > >> >         || !useless_type_conversion_p (lhs_type, rhs2_type))
> > >> >       {
> > >
> > > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > > index e321d929fd0..a845c7fff93 100644
> > > --- a/gcc/tree-cfg.cc
> > > +++ b/gcc/tree-cfg.cc
> > > @@ -4307,18 +4307,14 @@ verify_gimple_assign_ternary (gassign *stmt)
> > >        break;
> > >
> > >      case VEC_PERM_EXPR:
> > > -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > > -       || !useless_type_conversion_p (lhs_type, rhs2_type))
> > > -     {
> > > -       error ("type mismatch in %qs", code_name);
> > > -       debug_generic_expr (lhs_type);
> > > -       debug_generic_expr (rhs1_type);
> > > -       debug_generic_expr (rhs2_type);
> > > -       debug_generic_expr (rhs3_type);
> > > -       return true;
> > > -     }
> > > +      /* If permute is constant, then we allow for lhs and rhs
> > > +      to have different vector types, provided:
> > > +      (1) lhs, rhs1, rhs2 have same element type.
> > > +      (2) rhs3 vector has integer element type.
> > > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> > >
> > > -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > > +      if (TREE_CODE (lhs_type) != VECTOR_TYPE
> > > +       || TREE_CODE (rhs1_type) != VECTOR_TYPE
> > >         || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > >         || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> > >       {
> > > @@ -4330,10 +4326,29 @@ verify_gimple_assign_ternary (gassign *stmt)
> > >         return true;
> > >       }
> > >
> > > +      /* If lhs, rhs1, and rhs2 are different vector types,
> > > +      then relax the check if rhs3 is constant and lhs, rhs1, and rhs2
> > > +      have same element types.  */
> > > +      if ((!useless_type_conversion_p (lhs_type, rhs1_type)
> > > +        || !useless_type_conversion_p (lhs_type, rhs2_type))
> > > +       && (!TREE_CONSTANT (rhs3)
> > > +           || TREE_TYPE (lhs_type) != TREE_TYPE (rhs1_type)
> > > +           || TREE_TYPE (lhs_type) != TREE_TYPE (rhs2_type)))
> >
> > These TREE_TYPE tests should use !useless_type_conversion_p too,
> > instead of !=.  Maybe it would be easier to follow as:
> >
> >   if (TREE_CONSTANT (rhs3)
> >       ? ...
> >       : ...)
> >
> > so that we don't have doubled useless_type_conversion_p checks
> > for the TREE_CONSTANT case.
> >
> > > +     {
> > > +         error ("type mismatch in %qs", code_name);
> > > +         debug_generic_expr (lhs_type);
> > > +         debug_generic_expr (rhs1_type);
> > > +         debug_generic_expr (rhs2_type);
> > > +         debug_generic_expr (rhs3_type);
> > > +         return true;
> > > +     }
> > > +
> > > +      /* If rhs3 is constant, relax the check len(rhs2) == len(rhs3).  */
> > >        if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> > >                   TYPE_VECTOR_SUBPARTS (rhs2_type))
> > > -       || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > > +       || (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > >                      TYPE_VECTOR_SUBPARTS (rhs3_type))
> > > +           && !TREE_CONSTANT (rhs3))
> >
> > Very minor, but I think this reads better with the !TREE_CONSTANT first
> > in the && rather than second.  There's no reason to compare the lengths
> > for TREE_CONSTANT.
> >
> > Otherwise it looks good to me, but Richard should have the final say.
> Thanks, I addressed the above suggestions in the attached patch.
> Does it look OK ?
ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594349.html
Patch passes bootstrap+test on x86_64-linux-gnu.
On aarch64-linux-gnu, the patch passes bootstrap, but I am seeing
these discrepancies in test results:

New tests that FAIL (2 tests):

g++.dg/modules/macloc-1_c.C -std=c++17  dg-regexp 13 not found:
"[^\\n]*macloc-1_c.C:8:7: error: too many arguments to function 'int
you@agnes\\(\\)'\\nIn module agnes, imported at
[^\\n]*macloc-1_b.C:8,\\nof module edith, imported at
[^\\n]*macloc-1_c.C:3:\\n[^\\n]*macloc-1_a.C:12:14: note: declared
here\\n[^\\n]*macloc-1_a.C:9:22: note: in definition of macro
'KEVIN'\\n"
g++.dg/modules/macloc-1_c.C -std=c++17 (test for excess errors)

Old tests that failed, that have disappeared (2 tests): (Eeek!)

c-c++-common/missing-header-3.c  -Wc++-compat   expected multiline
pattern lines 5-7 not found: "\\s*#import
<this-file-does-not-exist\\.h>[^\\n\\r]*\\n
\\^~~~~~~~~~~~~~~~~~~~~~~~~~~~\\ncompilation
terminated\\.[^\\n\\r]*\\n"
c-c++-common/missing-header-3.c  -Wc++-compat  (test for excess errors)

I am not sure if these seem related to patch tho ?

Thanks,
Prathamesh
>
> Thanks,
> Prathamesh
> >
> > Thanks,
> > Richard
> >
> > >         || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> > >                      TYPE_VECTOR_SUBPARTS (lhs_type)))
> > >       {
  

On Mon, 23 May 2022 at 22:57, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
>
> On Mon, 9 May 2022 at 21:21, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
> >
> > On Mon, 9 May 2022 at 19:22, Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> > >
> > > Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > > > On Tue, 3 May 2022 at 18:25, Richard Sandiford
> > > > <richard.sandiford@arm.com> wrote:
> > > >>
> > > >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > > >> > On Tue, 4 Jan 2022 at 19:12, Richard Sandiford
> > > >> > <richard.sandiford@arm.com> wrote:
> > > >> >>
> > > >> >> Richard Biener <rguenther@suse.de> writes:
> > > >> >> > On Tue, 4 Jan 2022, Richard Sandiford wrote:
> > > >> >> >
> > > >> >> >> Richard Biener <rguenther@suse.de> writes:
> > > >> >> >> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
> > > >> >> >> >
> > > >> >> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > > >> >> >> >> > Hi,
> > > >> >> >> >> > The attached patch rearranges order of type-check for vec_perm_expr
> > > >> >> >> >> > and relaxes type checking for
> > > >> >> >> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> > > >> >> >> >> >
> > > >> >> >> >> > when:
> > > >> >> >> >> > rhs1 == rhs2,
> > > >> >> >> >> > lhs is variable length vector,
> > > >> >> >> >> > rhs1 is fixed length vector,
> > > >> >> >> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> > > >> >> >> >> >
> > > >> >> >> >> > I am not sure tho if this check is correct ? My intent was to capture
> > > >> >> >> >> > case when vec_perm_expr is used to "extend" fixed length vector to
> > > >> >> >> >> > it's VLA equivalent.
> > > >> >> >> >>
> > > >> >> >> >> VLAness isn't really the issue.  We want the same thing to work for
> > > >> >> >> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> > > >> >> >> >> vectors are fixed-length in that case.
> > > >> >> >> >>
> > > >> >> >> >> The principle is that for:
> > > >> >> >> >>
> > > >> >> >> >>   A = VEC_PERM_EXPR <B, C, D>;
> > > >> >> >> >>
> > > >> >> >> >> the requirements are:
> > > >> >> >> >>
> > > >> >> >> >> - A, B, C and D must be vectors
> > > >> >> >> >> - A, B and C must have the same element type
> > > >> >> >> >> - D must have an integer element type
> > > >> >> >> >> - A and D must have the same number of elements (NA)
> > > >> >> >> >> - B and C must have the same number of elements (NB)
> > > >> >> >> >>
> > > >> >> >> >> The semantics are that we create a joined vector BC (all elements of B
> > > >> >> >> >> followed by all element of C) and that:
> > > >> >> >> >>
> > > >> >> >> >>   A[i] = BC[D[i] % (NB+NB)]
> > > >> >> >> >>
> > > >> >> >> >> for 0 ≤ i < NA.
> > > >> >> >> >>
> > > >> >> >> >> This operation makes sense even if NA != NB.
> > > >> >> >> >
> > > >> >> >> > But note that we don't currently expect NA != NB and the optab just
> > > >> >> >> > has a single mode.
> > > >> >> >>
> > > >> >> >> True, but we only need this for constant permutes.  They are already
> > > >> >> >> special in that they allow the index elements to be wider than the data
> > > >> >> >> elements.
> > > >> >> >
> > > >> >> > OK, then we should reflect this in the stmt verification and only relax
> > > >> >> > the constant permute vector case and also amend the
> > > >> >> > TARGET_VECTORIZE_VEC_PERM_CONST accordingly.
> > > >> >>
> > > >> >> Sounds good.
> > > >> >>
> > > >> >> > For non-constant permutes the docs say the mode of vec_perm is
> > > >> >> > the common mode of operands 1 and 2 whilst the mode of operand 0
> > > >> >> > is unspecified - even unconstrained by the docs.  I'm not sure
> > > >> >> > if vec_perm expansion is expected to eventually FAIL.  Updating the
> > > >> >> > docs of vec_perm would be appreciated as well.
> > > >> >>
> > > >> >> Yeah, I guess de facto operand 0 has to be the same mode as operands
> > > >> >> 1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
> > > >> >> or self-explanatory at the time. :-)
> > > >> >>
> > > >> >> > As said I prefer to not mangle the existing stmt checking too much
> > > >> >> > at this stage so minimal adjustment is prefered there.
> > > >> >>
> > > >> >> The PR is only an enhancement request rather than a bug, so I think the
> > > >> >> patch would need to wait for GCC 13 whatever happens.
> > > >> > Hi,
> > > >> > In attached patch, the type checking is relaxed only if mask is constant.
> > > >> > Does this look OK ?
> > > >> >
> > > >> > Thanks,
> > > >> > Prathamesh
> > > >> >>
> > > >> >> Thanks,
> > > >> >> Richard
> > > >> >
> > > >> > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > > >> > index e321d929fd0..02b88f67855 100644
> > > >> > --- a/gcc/tree-cfg.cc
> > > >> > +++ b/gcc/tree-cfg.cc
> > > >> > @@ -4307,6 +4307,24 @@ verify_gimple_assign_ternary (gassign *stmt)
> > > >> >        break;
> > > >> >
> > > >> >      case VEC_PERM_EXPR:
> > > >> > +      /* If permute is constant, then we allow for lhs and rhs
> > > >> > +      to have different vector types, provided:
> > > >> > +      (1) lhs, rhs1, rhs2, and rhs3 have same element type.
> > > >>
> > > >> This isn't a requirement for rhs3.
> > > >>
> > > >> > +      (2) rhs3 vector has integer element type.
> > > >> > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> > > >> > +
> > > >> > +      if (TREE_CONSTANT (rhs3)
> > > >> > +       && VECTOR_TYPE_P (lhs_type)
> > > >> > +       && VECTOR_TYPE_P (rhs1_type)
> > > >> > +       && VECTOR_TYPE_P (rhs2_type)
> > > >> > +       && VECTOR_TYPE_P (rhs3_type)
> > > >> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
> > > >> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
> > > >> > +       && INTEGRAL_TYPE_P (TREE_TYPE (rhs3_type))
> > > >> > +       && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
> > > >> > +       && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
> > > >> > +     return false;
> > > >> > +
> > > >>
> > > >> I think this should be integrated into the existing conditions
> > > >> rather than done as an initial special case.
> > > >>
> > > >> It might make sense to start with:
> > > >>
> > > >>       if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > > >>           || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > > >>           || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> > > >>         {
> > > >>
> > > >> but expanded to test lhs_type too.  Then the other parts of the new test
> > > >> should be distributed across the existing conditions.
> > > >>
> > > >> The type tests should use useless_type_conversion_p rather than ==.
> > > > Hi Richard,
> > > > Thanks for the suggestions. In the attached patch, I tried to
> > > > distribute the checks
> > > > across existing conditions, does it look OK ?
> > > > I am not sure how to write tests for the type checks tho, does
> > > > gimple-fe support vec_perm_expr ?
> > > > I did a quick testsuite run for vect.exp and the patch doesn't seem to
> > > > cause any unexpected failures.
> > > >
> > > > Thanks,
> > > > Prathamesh
> > > >>
> > > >> Thanks,
> > > >> Richard
> > > >>
> > > >>
> > > >>
> > > >> >        if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > > >> >         || !useless_type_conversion_p (lhs_type, rhs2_type))
> > > >> >       {
> > > >
> > > > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > > > index e321d929fd0..a845c7fff93 100644
> > > > --- a/gcc/tree-cfg.cc
> > > > +++ b/gcc/tree-cfg.cc
> > > > @@ -4307,18 +4307,14 @@ verify_gimple_assign_ternary (gassign *stmt)
> > > >        break;
> > > >
> > > >      case VEC_PERM_EXPR:
> > > > -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > > > -       || !useless_type_conversion_p (lhs_type, rhs2_type))
> > > > -     {
> > > > -       error ("type mismatch in %qs", code_name);
> > > > -       debug_generic_expr (lhs_type);
> > > > -       debug_generic_expr (rhs1_type);
> > > > -       debug_generic_expr (rhs2_type);
> > > > -       debug_generic_expr (rhs3_type);
> > > > -       return true;
> > > > -     }
> > > > +      /* If permute is constant, then we allow for lhs and rhs
> > > > +      to have different vector types, provided:
> > > > +      (1) lhs, rhs1, rhs2 have same element type.
> > > > +      (2) rhs3 vector has integer element type.
> > > > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> > > >
> > > > -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > > > +      if (TREE_CODE (lhs_type) != VECTOR_TYPE
> > > > +       || TREE_CODE (rhs1_type) != VECTOR_TYPE
> > > >         || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > > >         || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> > > >       {
> > > > @@ -4330,10 +4326,29 @@ verify_gimple_assign_ternary (gassign *stmt)
> > > >         return true;
> > > >       }
> > > >
> > > > +      /* If lhs, rhs1, and rhs2 are different vector types,
> > > > +      then relax the check if rhs3 is constant and lhs, rhs1, and rhs2
> > > > +      have same element types.  */
> > > > +      if ((!useless_type_conversion_p (lhs_type, rhs1_type)
> > > > +        || !useless_type_conversion_p (lhs_type, rhs2_type))
> > > > +       && (!TREE_CONSTANT (rhs3)
> > > > +           || TREE_TYPE (lhs_type) != TREE_TYPE (rhs1_type)
> > > > +           || TREE_TYPE (lhs_type) != TREE_TYPE (rhs2_type)))
> > >
> > > These TREE_TYPE tests should use !useless_type_conversion_p too,
> > > instead of !=.  Maybe it would be easier to follow as:
> > >
> > >   if (TREE_CONSTANT (rhs3)
> > >       ? ...
> > >       : ...)
> > >
> > > so that we don't have doubled useless_type_conversion_p checks
> > > for the TREE_CONSTANT case.
> > >
> > > > +     {
> > > > +         error ("type mismatch in %qs", code_name);
> > > > +         debug_generic_expr (lhs_type);
> > > > +         debug_generic_expr (rhs1_type);
> > > > +         debug_generic_expr (rhs2_type);
> > > > +         debug_generic_expr (rhs3_type);
> > > > +         return true;
> > > > +     }
> > > > +
> > > > +      /* If rhs3 is constant, relax the check len(rhs2) == len(rhs3).  */
> > > >        if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> > > >                   TYPE_VECTOR_SUBPARTS (rhs2_type))
> > > > -       || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > > > +       || (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > > >                      TYPE_VECTOR_SUBPARTS (rhs3_type))
> > > > +           && !TREE_CONSTANT (rhs3))
> > >
> > > Very minor, but I think this reads better with the !TREE_CONSTANT first
> > > in the && rather than second.  There's no reason to compare the lengths
> > > for TREE_CONSTANT.
> > >
> > > Otherwise it looks good to me, but Richard should have the final say.
> > Thanks, I addressed the above suggestions in the attached patch.
> > Does it look OK ?
> ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594349.html
> Patch passes bootstrap+test on x86_64-linux-gnu.
> On aarch64-linux-gnu, the patch passes bootstrap, but I am seeing
> these discrepancies in test results:
>
> New tests that FAIL (2 tests):
>
> g++.dg/modules/macloc-1_c.C -std=c++17  dg-regexp 13 not found:
> "[^\\n]*macloc-1_c.C:8:7: error: too many arguments to function 'int
> you@agnes\\(\\)'\\nIn module agnes, imported at
> [^\\n]*macloc-1_b.C:8,\\nof module edith, imported at
> [^\\n]*macloc-1_c.C:3:\\n[^\\n]*macloc-1_a.C:12:14: note: declared
> here\\n[^\\n]*macloc-1_a.C:9:22: note: in definition of macro
> 'KEVIN'\\n"
> g++.dg/modules/macloc-1_c.C -std=c++17 (test for excess errors)
>
> Old tests that failed, that have disappeared (2 tests): (Eeek!)
>
> c-c++-common/missing-header-3.c  -Wc++-compat   expected multiline
> pattern lines 5-7 not found: "\\s*#import
> <this-file-does-not-exist\\.h>[^\\n\\r]*\\n
> \\^~~~~~~~~~~~~~~~~~~~~~~~~~~~\\ncompilation
> terminated\\.[^\\n\\r]*\\n"
> c-c++-common/missing-header-3.c  -Wc++-compat  (test for excess errors)
>
> I am not sure if these seem related to patch tho ?
ping * 2 https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594349.html

Thanks,
Prathamesh
>
> Thanks,
> Prathamesh
> >
> > Thanks,
> > Prathamesh
> > >
> > > Thanks,
> > > Richard
> > >
> > > >         || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs3_type),
> > > >                      TYPE_VECTOR_SUBPARTS (lhs_type)))
> > > >       {
  

On Tue, 31 May 2022, Prathamesh Kulkarni wrote:

> On Mon, 23 May 2022 at 22:57, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
> >
> > On Mon, 9 May 2022 at 21:21, Prathamesh Kulkarni
> > <prathamesh.kulkarni@linaro.org> wrote:
> > >
> > > On Mon, 9 May 2022 at 19:22, Richard Sandiford
> > > <richard.sandiford@arm.com> wrote:
> > > >
> > > > Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > > > > On Tue, 3 May 2022 at 18:25, Richard Sandiford
> > > > > <richard.sandiford@arm.com> wrote:
> > > > >>
> > > > >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > > > >> > On Tue, 4 Jan 2022 at 19:12, Richard Sandiford
> > > > >> > <richard.sandiford@arm.com> wrote:
> > > > >> >>
> > > > >> >> Richard Biener <rguenther@suse.de> writes:
> > > > >> >> > On Tue, 4 Jan 2022, Richard Sandiford wrote:
> > > > >> >> >
> > > > >> >> >> Richard Biener <rguenther@suse.de> writes:
> > > > >> >> >> > On Fri, 17 Dec 2021, Richard Sandiford wrote:
> > > > >> >> >> >
> > > > >> >> >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> > > > >> >> >> >> > Hi,
> > > > >> >> >> >> > The attached patch rearranges order of type-check for vec_perm_expr
> > > > >> >> >> >> > and relaxes type checking for
> > > > >> >> >> >> > lhs = vec_perm_expr<rhs1, rhs2, mask>
> > > > >> >> >> >> >
> > > > >> >> >> >> > when:
> > > > >> >> >> >> > rhs1 == rhs2,
> > > > >> >> >> >> > lhs is variable length vector,
> > > > >> >> >> >> > rhs1 is fixed length vector,
> > > > >> >> >> >> > TREE_TYPE (lhs) == TREE_TYPE (rhs1)
> > > > >> >> >> >> >
> > > > >> >> >> >> > I am not sure tho if this check is correct ? My intent was to capture
> > > > >> >> >> >> > case when vec_perm_expr is used to "extend" fixed length vector to
> > > > >> >> >> >> > it's VLA equivalent.
> > > > >> >> >> >>
> > > > >> >> >> >> VLAness isn't really the issue.  We want the same thing to work for
> > > > >> >> >> >> -msve-vector-bits=256, -msve-vector-bits=512, etc., even though the
> > > > >> >> >> >> vectors are fixed-length in that case.
> > > > >> >> >> >>
> > > > >> >> >> >> The principle is that for:
> > > > >> >> >> >>
> > > > >> >> >> >>   A = VEC_PERM_EXPR <B, C, D>;
> > > > >> >> >> >>
> > > > >> >> >> >> the requirements are:
> > > > >> >> >> >>
> > > > >> >> >> >> - A, B, C and D must be vectors
> > > > >> >> >> >> - A, B and C must have the same element type
> > > > >> >> >> >> - D must have an integer element type
> > > > >> >> >> >> - A and D must have the same number of elements (NA)
> > > > >> >> >> >> - B and C must have the same number of elements (NB)
> > > > >> >> >> >>
> > > > >> >> >> >> The semantics are that we create a joined vector BC (all elements of B
> > > > >> >> >> >> followed by all element of C) and that:
> > > > >> >> >> >>
> > > > >> >> >> >>   A[i] = BC[D[i] % (NB+NB)]
> > > > >> >> >> >>
> > > > >> >> >> >> for 0 ? i < NA.
> > > > >> >> >> >>
> > > > >> >> >> >> This operation makes sense even if NA != NB.
> > > > >> >> >> >
> > > > >> >> >> > But note that we don't currently expect NA != NB and the optab just
> > > > >> >> >> > has a single mode.
> > > > >> >> >>
> > > > >> >> >> True, but we only need this for constant permutes.  They are already
> > > > >> >> >> special in that they allow the index elements to be wider than the data
> > > > >> >> >> elements.
> > > > >> >> >
> > > > >> >> > OK, then we should reflect this in the stmt verification and only relax
> > > > >> >> > the constant permute vector case and also amend the
> > > > >> >> > TARGET_VECTORIZE_VEC_PERM_CONST accordingly.
> > > > >> >>
> > > > >> >> Sounds good.
> > > > >> >>
> > > > >> >> > For non-constant permutes the docs say the mode of vec_perm is
> > > > >> >> > the common mode of operands 1 and 2 whilst the mode of operand 0
> > > > >> >> > is unspecified - even unconstrained by the docs.  I'm not sure
> > > > >> >> > if vec_perm expansion is expected to eventually FAIL.  Updating the
> > > > >> >> > docs of vec_perm would be appreciated as well.
> > > > >> >>
> > > > >> >> Yeah, I guess de facto operand 0 has to be the same mode as operands
> > > > >> >> 1 and 2.  Maybe that was just an oversight, or maybe it seemed obvious
> > > > >> >> or self-explanatory at the time. :-)
> > > > >> >>
> > > > >> >> > As said I prefer to not mangle the existing stmt checking too much
> > > > >> >> > at this stage so minimal adjustment is prefered there.
> > > > >> >>
> > > > >> >> The PR is only an enhancement request rather than a bug, so I think the
> > > > >> >> patch would need to wait for GCC 13 whatever happens.
> > > > >> > Hi,
> > > > >> > In attached patch, the type checking is relaxed only if mask is constant.
> > > > >> > Does this look OK ?
> > > > >> >
> > > > >> > Thanks,
> > > > >> > Prathamesh
> > > > >> >>
> > > > >> >> Thanks,
> > > > >> >> Richard
> > > > >> >
> > > > >> > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > > > >> > index e321d929fd0..02b88f67855 100644
> > > > >> > --- a/gcc/tree-cfg.cc
> > > > >> > +++ b/gcc/tree-cfg.cc
> > > > >> > @@ -4307,6 +4307,24 @@ verify_gimple_assign_ternary (gassign *stmt)
> > > > >> >        break;
> > > > >> >
> > > > >> >      case VEC_PERM_EXPR:
> > > > >> > +      /* If permute is constant, then we allow for lhs and rhs
> > > > >> > +      to have different vector types, provided:
> > > > >> > +      (1) lhs, rhs1, rhs2, and rhs3 have same element type.
> > > > >>
> > > > >> This isn't a requirement for rhs3.
> > > > >>
> > > > >> > +      (2) rhs3 vector has integer element type.
> > > > >> > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> > > > >> > +
> > > > >> > +      if (TREE_CONSTANT (rhs3)
> > > > >> > +       && VECTOR_TYPE_P (lhs_type)
> > > > >> > +       && VECTOR_TYPE_P (rhs1_type)
> > > > >> > +       && VECTOR_TYPE_P (rhs2_type)
> > > > >> > +       && VECTOR_TYPE_P (rhs3_type)
> > > > >> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs1_type)
> > > > >> > +       && TREE_TYPE (lhs_type) == TREE_TYPE (rhs2_type)
> > > > >> > +       && INTEGRAL_TYPE_P (TREE_TYPE (rhs3_type))
> > > > >> > +       && known_eq (TYPE_VECTOR_SUBPARTS (lhs_type), TYPE_VECTOR_SUBPARTS (rhs3_type))
> > > > >> > +       && known_eq (TYPE_VECTOR_SUBPARTS (rhs1_type), TYPE_VECTOR_SUBPARTS (rhs2_type)))
> > > > >> > +     return false;
> > > > >> > +
> > > > >>
> > > > >> I think this should be integrated into the existing conditions
> > > > >> rather than done as an initial special case.
> > > > >>
> > > > >> It might make sense to start with:
> > > > >>
> > > > >>       if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > > > >>           || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > > > >>           || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> > > > >>         {
> > > > >>
> > > > >> but expanded to test lhs_type too.  Then the other parts of the new test
> > > > >> should be distributed across the existing conditions.
> > > > >>
> > > > >> The type tests should use useless_type_conversion_p rather than ==.
> > > > > Hi Richard,
> > > > > Thanks for the suggestions. In the attached patch, I tried to
> > > > > distribute the checks
> > > > > across existing conditions, does it look OK ?
> > > > > I am not sure how to write tests for the type checks tho, does
> > > > > gimple-fe support vec_perm_expr ?
> > > > > I did a quick testsuite run for vect.exp and the patch doesn't seem to
> > > > > cause any unexpected failures.
> > > > >
> > > > > Thanks,
> > > > > Prathamesh
> > > > >>
> > > > >> Thanks,
> > > > >> Richard
> > > > >>
> > > > >>
> > > > >>
> > > > >> >        if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > > > >> >         || !useless_type_conversion_p (lhs_type, rhs2_type))
> > > > >> >       {
> > > > >
> > > > > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > > > > index e321d929fd0..a845c7fff93 100644
> > > > > --- a/gcc/tree-cfg.cc
> > > > > +++ b/gcc/tree-cfg.cc
> > > > > @@ -4307,18 +4307,14 @@ verify_gimple_assign_ternary (gassign *stmt)
> > > > >        break;
> > > > >
> > > > >      case VEC_PERM_EXPR:
> > > > > -      if (!useless_type_conversion_p (lhs_type, rhs1_type)
> > > > > -       || !useless_type_conversion_p (lhs_type, rhs2_type))
> > > > > -     {
> > > > > -       error ("type mismatch in %qs", code_name);
> > > > > -       debug_generic_expr (lhs_type);
> > > > > -       debug_generic_expr (rhs1_type);
> > > > > -       debug_generic_expr (rhs2_type);
> > > > > -       debug_generic_expr (rhs3_type);
> > > > > -       return true;
> > > > > -     }
> > > > > +      /* If permute is constant, then we allow for lhs and rhs
> > > > > +      to have different vector types, provided:
> > > > > +      (1) lhs, rhs1, rhs2 have same element type.
> > > > > +      (2) rhs3 vector has integer element type.
> > > > > +      (3) len(lhs) == len(rhs3) && len(rhs1) == len(rhs2).  */
> > > > >
> > > > > -      if (TREE_CODE (rhs1_type) != VECTOR_TYPE
> > > > > +      if (TREE_CODE (lhs_type) != VECTOR_TYPE
> > > > > +       || TREE_CODE (rhs1_type) != VECTOR_TYPE
> > > > >         || TREE_CODE (rhs2_type) != VECTOR_TYPE
> > > > >         || TREE_CODE (rhs3_type) != VECTOR_TYPE)
> > > > >       {
> > > > > @@ -4330,10 +4326,29 @@ verify_gimple_assign_ternary (gassign *stmt)
> > > > >         return true;
> > > > >       }
> > > > >
> > > > > +      /* If lhs, rhs1, and rhs2 are different vector types,
> > > > > +      then relax the check if rhs3 is constant and lhs, rhs1, and rhs2
> > > > > +      have same element types.  */
> > > > > +      if ((!useless_type_conversion_p (lhs_type, rhs1_type)
> > > > > +        || !useless_type_conversion_p (lhs_type, rhs2_type))
> > > > > +       && (!TREE_CONSTANT (rhs3)
> > > > > +           || TREE_TYPE (lhs_type) != TREE_TYPE (rhs1_type)
> > > > > +           || TREE_TYPE (lhs_type) != TREE_TYPE (rhs2_type)))
> > > >
> > > > These TREE_TYPE tests should use !useless_type_conversion_p too,
> > > > instead of !=.  Maybe it would be easier to follow as:
> > > >
> > > >   if (TREE_CONSTANT (rhs3)
> > > >       ? ...
> > > >       : ...)
> > > >
> > > > so that we don't have doubled useless_type_conversion_p checks
> > > > for the TREE_CONSTANT case.
> > > >
> > > > > +     {
> > > > > +         error ("type mismatch in %qs", code_name);
> > > > > +         debug_generic_expr (lhs_type);
> > > > > +         debug_generic_expr (rhs1_type);
> > > > > +         debug_generic_expr (rhs2_type);
> > > > > +         debug_generic_expr (rhs3_type);
> > > > > +         return true;
> > > > > +     }
> > > > > +
> > > > > +      /* If rhs3 is constant, relax the check len(rhs2) == len(rhs3).  */
> > > > >        if (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs1_type),
> > > > >                   TYPE_VECTOR_SUBPARTS (rhs2_type))
> > > > > -       || maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > > > > +       || (maybe_ne (TYPE_VECTOR_SUBPARTS (rhs2_type),
> > > > >                      TYPE_VECTOR_SUBPARTS (rhs3_type))
> > > > > +           && !TREE_CONSTANT (rhs3))
> > > >
> > > > Very minor, but I think this reads better with the !TREE_CONSTANT first
> > > > in the && rather than second.  There's no reason to compare the lengths
> > > > for TREE_CONSTANT.
> > > >
> > > > Otherwise it looks good to me, but Richard should have the final say.
> > > Thanks, I addressed the above suggestions in the attached patch.
> > > Does it look OK ?
> > ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594349.html
> > Patch passes bootstrap+test on x86_64-linux-gnu.
> > On aarch64-linux-gnu, the patch passes bootstrap, but I am seeing
> > these discrepancies in test results:
> >
> > New tests that FAIL (2 tests):
> >
> > g++.dg/modules/macloc-1_c.C -std=c++17  dg-regexp 13 not found:
> > "[^\\n]*macloc-1_c.C:8:7: error: too many arguments to function 'int
> > you@agnes\\(\\)'\\nIn module agnes, imported at
> > [^\\n]*macloc-1_b.C:8,\\nof module edith, imported at
> > [^\\n]*macloc-1_c.C:3:\\n[^\\n]*macloc-1_a.C:12:14: note: declared
> > here\\n[^\\n]*macloc-1_a.C:9:22: note: in definition of macro
> > 'KEVIN'\\n"
> > g++.dg/modules/macloc-1_c.C -std=c++17 (test for excess errors)
> >
> > Old tests that failed, that have disappeared (2 tests): (Eeek!)
> >
> > c-c++-common/missing-header-3.c  -Wc++-compat   expected multiline
> > pattern lines 5-7 not found: "\\s*#import
> > <this-file-does-not-exist\\.h>[^\\n\\r]*\\n
> > \\^~~~~~~~~~~~~~~~~~~~~~~~~~~~\\ncompilation
> > terminated\\.[^\\n\\r]*\\n"
> > c-c++-common/missing-header-3.c  -Wc++-compat  (test for excess errors)
> >
> > I am not sure if these seem related to patch tho ?
> ping * 2 https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594349.html

OK.

Richard.