Re: [RFC] Add support for vectors in comparisons (like the C++ frontend does)

[Paul Iannetta <piannetta@kalrayinc.com>]

Hi Ian,

Observations are also very welcomed, thank you!

On Mon, Oct 10, 2022 at 03:37:24PM +0100, Iain Sandoe wrote:
> Hi Paul,
> 
> Not a review of the patch - but a couple of observations.
> 
> > On 10 Oct 2022, at 15:11, Paul Iannetta via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> > I am trying to bridge the gap between the extensions supported by the C
> > and C++ front-ends.  When it comes to vector extensions, C++ supports
> > vectors in comparisons and within conditional expressions whereas the C
> > front-end does not.
> 
> Equivalence seems, on the face of it, a reasonable objective - but I am
> curious as whether there is some more concrete motivation for the patch,
> e.g. some codebase that currently does not work but would with this change?
> 

The main motivation behind the equivalence is that, we have C and C++
codebases, and it is not very convenient to have to remember which
extension is allowed in C and not in C++ and vice-versa.  And, in this
case, it makes it harder for GCC to generate conditional moves for
code using vectors, especially since `a ? b : c` is not recognized,
and thus, we cannot rely on the `VEC_COND_EXPR` construction.

> > I have a patch to bring this feature to the C front-end as well, and
> > would like to hear your opinion on it, especially since it may affect
> > the feature-set of the objc front-end as well.
> 
> Likewise, I am interested in the motivation for the ObjC change.  The usual
> initial filter for consideration of functional changes (at least for the NeXT
> runtime on Darwin) is “What does current clang do?” or, even better, “what
> does current Xcode do?”.   There are several (possibly many) cases where
> C in clang has extensions to C++ behaviour - and vice-versa (because there
> is a single  front-end codebase, that happens easily, whether by design o
>  accident).

Well, the only motivation was that it was easier to make the change
for both front-ends at the same time and avoided some checks.  I'll
add them so that it won't introduce a divergence on what is accepted
by the objc front-end.

Thanks,
Paul

> 
> The reason for the ‘clang litmus test’ is that the effective language standard
> for ObjectiveC is determined by that compiler.
> 
> cheers
> Iain
> 
> > I have tried to mirror as much as possible what the C++ front-end does
> > and checked that both front-end produce the same GIMPLE for all the
> > simple expressions as well as some more complex combinations of the
> > operators (?:, !, ^, || and &&).
> > 
> > Currently, this is only a tentative patch and I did not add any tests
> > to the testsuite.  Moreover, the aarch64's target-specific testsuite
> > explicitly tests the non-presence of this feature, which will have to
> > be removed.
> > 
> > I've run the testsuite on x86 and I've not seen any regressions.
> > 
> > Cheers,
> > Paul
> > 
> > # ------------------------ >8 ------------------------
> > Support for vector types in simple comparisons
> > 
> > gcc/
> > 
> > 	* doc/extend.texi: Remove the C++ mention, since both C and C++
> > 	  support the all the mentioned features.
> > 
> > gcc/c/
> > 
> > 	* c-typeck.cc (build_unary_op): Add support for vector for the
> > 	  unary exclamation mark.
> > 	(build_conditional_expr): Add support for vector in conditional
> > 	expressions.
> > 	(build_binary_op): Add support for vector for &&, || and ^.
> > 	(c_objc_common_truthvalue_conversion): Remove the special gards
> > 	preventing vector types.
> > 
> > # ------------------------ >8 ------------------------
> > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> > index 17185fd3da4..03ade14cae9 100644
> > --- a/gcc/c/c-typeck.cc
> > +++ b/gcc/c/c-typeck.cc
> > @@ -4536,12 +4536,15 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
> >     case TRUTH_NOT_EXPR:
> >       if (typecode != INTEGER_TYPE && typecode != FIXED_POINT_TYPE
> > 	  && typecode != REAL_TYPE && typecode != POINTER_TYPE
> > -	  && typecode != COMPLEX_TYPE)
> > +	  && typecode != COMPLEX_TYPE && typecode != VECTOR_TYPE)
> > 	{
> > 	  error_at (location,
> > 		    "wrong type argument to unary exclamation mark");
> > 	  return error_mark_node;
> > 	}
> > +      if (gnu_vector_type_p (TREE_TYPE (arg)))
> > +	return build_binary_op (location, EQ_EXPR, arg,
> > +				build_zero_cst (TREE_TYPE (arg)), false);
> >       if (int_operands)
> > 	{
> > 	  arg = c_objc_common_truthvalue_conversion (location, xarg);
> > @@ -5477,6 +5480,129 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
> >       result_type = type2;
> >     }
> > 
> > +  if (gnu_vector_type_p (TREE_TYPE (ifexp))
> > +      && VECTOR_INTEGER_TYPE_P (TREE_TYPE (ifexp)))
> > +    {
> > +      tree ifexp_type = TREE_TYPE (ifexp);
> > +
> > +      /* If ifexp is another cond_expr choosing between -1 and 0,
> > +	 then we can use its comparison.  It may help to avoid
> > +	 additional comparison, produce more accurate diagnostics
> > +	 and enables folding.  */
> > +      if (TREE_CODE (ifexp) == VEC_COND_EXPR
> > +	  && integer_minus_onep (TREE_OPERAND (ifexp, 1))
> > +	  && integer_zerop (TREE_OPERAND (ifexp, 2)))
> > +	ifexp = TREE_OPERAND (ifexp, 0);
> > +
> > +      tree op1_type = TREE_TYPE (op1);
> > +      tree op2_type = TREE_TYPE (op2);
> > +
> > +      if (!VECTOR_TYPE_P (op1_type) && !VECTOR_TYPE_P (op2_type))
> > +	{
> > +	  /* Rely on the error messages of the scalar version.  */
> > +	  tree scal =
> > +	    build_conditional_expr (colon_loc, integer_one_node, ifexp_bcp,
> > +				    op1, op1_original_type, op1_loc,
> > +				    op2, op2_original_type, op2_loc);
> > +	  if (scal == error_mark_node)
> > +	    return error_mark_node;
> > +	  tree stype = TREE_TYPE (scal);
> > +	  tree ctype = TREE_TYPE (ifexp_type);
> > +	  if (TYPE_SIZE (stype) != TYPE_SIZE (ctype)
> > +	      || (!INTEGRAL_TYPE_P (stype) && !SCALAR_FLOAT_TYPE_P (stype)))
> > +	    {
> > +	      error_at (colon_loc,
> > +			"inferred scalar type %qT is not an integer or "
> > +			"floating-point type of the same size as %qT", stype,
> > +			COMPARISON_CLASS_P (ifexp)
> > +			? TREE_TYPE (TREE_TYPE (TREE_OPERAND (ifexp, 0)))
> > +			: ctype);
> > +	      return error_mark_node;
> > +	    }
> > +
> > +	  tree vtype = build_opaque_vector_type (stype,
> > +						 TYPE_VECTOR_SUBPARTS
> > +						 (ifexp_type));
> > +	  /* The warnings (like Wsign-conversion) have already been
> > +	     given by the scalar build_conditional_expr. We still check
> > +	     unsafe_conversion_p to forbid truncating long long -> float.  */
> > +	  if (unsafe_conversion_p (stype, op1, NULL_TREE, false))
> > +	    {
> > +	      error_at (colon_loc, "conversion of scalar %qT to vector %qT "
> > +			"involves truncation", op1_type, vtype);
> > +	      return error_mark_node;
> > +	    }
> > +	  if (unsafe_conversion_p (stype, op2, NULL_TREE, false))
> > +	    {
> > +	      error_at (colon_loc, "conversion of scalar %qT to vector %qT "
> > +			"involves truncation", op2_type, vtype);
> > +	      return error_mark_node;
> > +	    }
> > +
> > +	  op1 = convert (stype, op1);
> > +	  op1 = save_expr (op1);
> > +	  op1 = build_vector_from_val (vtype, op1);
> > +	  op1_type = vtype;
> > +	  op2 = convert (stype, op2);
> > +	  op2 = save_expr (op2);
> > +	  op2 = build_vector_from_val (vtype, op2);
> > +	  op2_type = vtype;
> > +	}
> > +
> > +      if (gnu_vector_type_p (op1_type) ^ gnu_vector_type_p (op2_type))
> > +	{
> > +	  enum stv_conv convert_flag =
> > +	    scalar_to_vector (colon_loc, VEC_COND_EXPR, op1, op2,
> > +			      true);
> > +
> > +	  switch (convert_flag)
> > +	    {
> > +	    case stv_error:
> > +	      return error_mark_node;
> > +	    case stv_firstarg:
> > +	      {
> > +		op1 = save_expr (op1);
> > +		op1 = convert (TREE_TYPE (op2_type), op1);
> > +		op1 = build_vector_from_val (op2_type, op1);
> > +		op1_type = TREE_TYPE (op1);
> > +		break;
> > +	      }
> > +	    case stv_secondarg:
> > +	      {
> > +		op2 = save_expr (op2);
> > +		op2 = convert (TREE_TYPE (op1_type), op2);
> > +		op2 = build_vector_from_val (op1_type, op2);
> > +		op2_type = TREE_TYPE (op2);
> > +		break;
> > +	      }
> > +	    default:
> > +	      break;
> > +	    }
> > +	}
> > +
> > +      if (!gnu_vector_type_p (op1_type)
> > +	  || !gnu_vector_type_p (op2_type)
> > +	  || !comptypes (op1_type, op2_type)
> > +	  || maybe_ne (TYPE_VECTOR_SUBPARTS (ifexp_type),
> > +		       TYPE_VECTOR_SUBPARTS (op1_type))
> > +	  || TYPE_SIZE (ifexp_type) != TYPE_SIZE (op1_type))
> > +	{
> > +	  error_at (colon_loc,
> > +		    "incompatible vector types in conditional expression: "
> > +		    "%qT, %qT and %qT", TREE_TYPE (ifexp),
> > +		    TREE_TYPE (orig_op1), TREE_TYPE (orig_op2));
> > +	  return error_mark_node;
> > +	}
> > +
> > +      if (!COMPARISON_CLASS_P (ifexp))
> > +	{
> > +	  tree cmp_type = truth_type_for (ifexp_type);
> > +	  ifexp = build2 (NE_EXPR, cmp_type, ifexp,
> > +			  build_zero_cst (ifexp_type));
> > +	}
> > +      return build3_loc (colon_loc, VEC_COND_EXPR, op1_type, ifexp, op1, op2);
> > +    }
> > +
> >   if (!result_type)
> >     {
> >       if (flag_cond_mismatch)
> > @@ -5522,17 +5648,6 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
> > 		       && !TREE_OVERFLOW (orig_op2)));
> >     }
> > 
> > -  /* Need to convert condition operand into a vector mask.  */
> > -  if (VECTOR_TYPE_P (TREE_TYPE (ifexp)))
> > -    {
> > -      tree vectype = TREE_TYPE (ifexp);
> > -      tree elem_type = TREE_TYPE (vectype);
> > -      tree zero = build_int_cst (elem_type, 0);
> > -      tree zero_vec = build_vector_from_val (vectype, zero);
> > -      tree cmp_type = truth_type_for (vectype);
> > -      ifexp = build2 (NE_EXPR, cmp_type, ifexp, zero_vec);
> > -    }
> > -
> >   if (int_const || (ifexp_bcp && TREE_CODE (ifexp) == INTEGER_CST))
> >     ret = fold_build3_loc (colon_loc, COND_EXPR, result_type, ifexp, op1, op2);
> >   else
> > @@ -12105,6 +12220,54 @@ build_binary_op (location_t location, enum tree_code code,
> > 		       && (op0 == truthvalue_true_node
> > 			   || !TREE_OVERFLOW (orig_op1)));
> > 	}
> > +      if (!VECTOR_TYPE_P (type0) && gnu_vector_type_p (type1))
> > +	{
> > +	  if (!COMPARISON_CLASS_P (op1))
> > +	    op1 = build_binary_op (EXPR_LOCATION (op1), NE_EXPR, op1,
> > +				   build_zero_cst (type1), false);
> > +	  if (code == TRUTH_ANDIF_EXPR)
> > +	    {
> > +	      tree z = build_zero_cst (TREE_TYPE (op1));
> > +	      return build_conditional_expr (location, op0, 0,
> > +					     op1, NULL_TREE, EXPR_LOCATION (op1),
> > +					     z, NULL_TREE, EXPR_LOCATION (z));
> > +	    }
> > +	  else if (code == TRUTH_ORIF_EXPR)
> > +	    {
> > +	      tree m1 = build_all_ones_cst (TREE_TYPE (op1));
> > +	      return build_conditional_expr (location, op0, 0,
> > +					      m1, NULL_TREE, EXPR_LOCATION (m1),
> > +					      op1, NULL_TREE, EXPR_LOCATION (op1));
> > +	    }
> > +	  else
> > +	    gcc_unreachable ();
> > +	}
> > +      if (gnu_vector_type_p (type0)
> > +	  && (!VECTOR_TYPE_P (type1) || gnu_vector_type_p (type1)))
> > +	{
> > +	  if (!COMPARISON_CLASS_P (op0))
> > +	    op0 = build_binary_op (EXPR_LOCATION (op0), NE_EXPR, op0,
> > +				      build_zero_cst (type0), false);
> > +	  if (!VECTOR_TYPE_P (type1))
> > +	    {
> > +	      tree m1 = build_all_ones_cst (TREE_TYPE (op0));
> > +	      tree z = build_zero_cst (TREE_TYPE (op0));
> > +	      op1 = build_conditional_expr (location, op1, 0,
> > +					    m1, NULL_TREE, EXPR_LOCATION (m1),
> > +					    z, NULL_TREE, EXPR_LOCATION(z));
> > +	    }
> > +	  else if (!COMPARISON_CLASS_P (op1))
> > +	    op1 = build_binary_op (EXPR_LOCATION (op1), NE_EXPR, op1,
> > +				   build_zero_cst (type1), false);
> > +	  if (code == TRUTH_ANDIF_EXPR)
> > +	    code = BIT_AND_EXPR;
> > +	  else if (code == TRUTH_ORIF_EXPR)
> > +	    code = BIT_IOR_EXPR;
> > +	  else
> > +	    gcc_unreachable ();
> > +
> > +	  return build_binary_op (location, code, op0, op1, false);
> > +	}
> >       break;
> > 
> >       /* Shift operations: result has same type as first operand;
> > @@ -12906,10 +13069,6 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
> >     case FUNCTION_TYPE:
> >       gcc_unreachable ();
> > 
> > -    case VECTOR_TYPE:
> > -      error_at (location, "used vector type where scalar is required");
> > -      return error_mark_node;
> > -
> >     default:
> >       break;
> >     }
> > @@ -12924,8 +13083,6 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
> >       expr = note_integer_operands (expr);
> >     }
> >   else
> > -    /* ??? Should we also give an error for vectors rather than leaving
> > -       those to give errors later?  */
> >     expr = c_common_truthvalue_conversion (location, expr);
> > 
> >   if (TREE_CODE (expr) == INTEGER_CST && int_operands && !int_const)
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index c89df8778b2..1e0d436c02c 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -12007,7 +12007,7 @@ c = a >  b;     /* The result would be @{0, 0,-1, 0@}  */
> > c = a == b;     /* The result would be @{0,-1, 0,-1@}  */
> > @end smallexample
> > 
> > -In C++, the ternary operator @code{?:} is available. @code{a?b:c}, where
> > +The ternary operator @code{?:} is available. @code{a?b:c}, where
> > @code{b} and @code{c} are vectors of the same type and @code{a} is an
> > integer vector with the same number of elements of the same size as @code{b}
> > and @code{c}, computes all three arguments and creates a vector
> > @@ -12020,7 +12020,7 @@ vector. If both @code{b} and @code{c} are scalars and the type of
> > @code{b} and @code{c} are converted to a vector type whose elements have
> > this type and with the same number of elements as @code{a}.
> > 
> > -In C++, the logic operators @code{!, &&, ||} are available for vectors.
> > +The logic operators @code{!, &&, ||} are available for vectors.
> > @code{!v} is equivalent to @code{v == 0}, @code{a && b} is equivalent to
> > @code{a!=0 & b!=0} and @code{a || b} is equivalent to @code{a!=0 | b!=0}.
> > For mixed operations between a scalar @code{s} and a vector @code{v},
> > 
> > 
> > 
> > 
> 
> 
> 
> 
>