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

[Iain Sandoe <iain@sandoe.co.uk>]

Hi Paul,

> On 10 Oct 2022, at 16:20, Paul Iannetta <piannetta@kalrayinc.com> wrote:

> 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.

I have no objection to GCC growing extensions to Objective-C (even ones that
clang does not have) provided that we check what clang does and (say under the
-pedantic flag) reject extensions that are not present there.  So there is no need
to back out of this - but some example codes that we can check would be useful. 

In general, extensions to the “standard” language would be behind some flag that
disables them when the “standard” version is selected - and gives warnings/errors
as appropriate for ‘-pedantic’.  So that, for example, -std=gnuCXX could enable the
extensions, but -std=CXX would complain.

thanks
Iain

> 
> 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},