diff mbox

vector conditional expression with scalar arguments

Message ID alpine.DEB.2.02.1307071343320.31289@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse July 7, 2013, 12:02 p.m. UTC 
Hello,

this patch lets us write (vec<0)?-1:1 without having to manually construct 
a vector of 1s. It seems preferable to the hacks we can currently use, 
like (vec!=vec)+1. The patch also fixes a few bugs along the way.

Bootstrap+testsuite on x86_64-unknown-linux-gnu.

2013-07-07  Marc Glisse  <marc.glisse@inria.fr>

gcc/cp/
 	* call.c (build_conditional_expr_1): Handle the case with 1 vector
 	and 2 scalars. Call save_expr before building a vector.
 	* typeck.c (cp_build_binary_op): Check complain before complaining.

gcc/testsuite/
 	* g++.dg/ext/vector19.C: Adapt.
 	* g++.dg/ext/vector23.C: New testcase.

Comments

Jason Merrill July 8, 2013, 4:04 p.m. UTC | #1 
On 07/07/2013 08:02 AM, Marc Glisse wrote:
> +		    error_at (loc, "could not find an integer type "
> +				   "of the same size as %qT", ctype);

Why try to find a result element type the same size as the condition 
element type?  For scalars the condition is bool and the result can be 
any type.

Jason
Marc Glisse July 8, 2013, 9:28 p.m. UTC | #2 
On Mon, 8 Jul 2013, Jason Merrill wrote:

> On 07/07/2013 08:02 AM, Marc Glisse wrote:
>> +		    error_at (loc, "could not find an integer type "
>> +				   "of the same size as %qT", ctype);

Now that I think of it, if a?b:c fails for this reason, it would also fail 
later because it can't compute a!=0 (which is what the first argument is 
expanded to), so I could move the construction of a!=0 earlier instead, 
and re-use its type (modulo signed/unsigned).

> Why try to find a result element type the same size as the condition element 
> type?  For scalars the condition is bool and the result can be any type.

It is a guess, so it may be wrong. My experience on x86 is that vectors 
have a fixed size (128 bits for SSE, 256 for AVX) and most operations act 
on vectors of the same size. Mixing vectors of different sizes is likely 
to result in operations not supported by the hardware and expanded to slow 
scalar code. Comparisons already return a signed integer vector type of 
the same size as the operands (so they fail for vectors of long double on 
x86).

Now there are some exceptions on x86, and it may be that other 
architectures have more. The vectorizer even has code to handle these 
mixed sizes. We could also consider that selecting a better vector size is 
the role of a middle-end optimization and should not affect the language 
rules.

Note that the VEC_COND_EXPR we currently have in gcc requires the 3 
arguments to have the same size and number of elements, so we would need 
to use VEC_(UN)PACK_*_EXPR and CONSTRUCTOR/BIT_FIELD_REF (all our tree 
codes are for vectors of fixed size, so doubling the size of the elements 
also drops half the elements).

Perhaps the most conservative rule would be to only accept the case where 
the sizes match and reject the others for now, so whatever is decided 
later for other cases is unlikely to require a breaking change. Though I 
would like the general case to be accepted eventually, whatever it ends up 
meaning ;-)
Jason Merrill July 9, 2013, 3:12 a.m. UTC | #3 
On 07/08/2013 05:28 PM, Marc Glisse wrote:
> Perhaps the most conservative rule would be to only accept the case
> where the sizes match and reject the others for now, so whatever is
> decided later for other cases is unlikely to require a breaking change.
> Though I would like the general case to be accepted eventually, whatever
> it ends up meaning :)

Makes sense to me.

Jason
diff mbox

Patch

Index: testsuite/g++.dg/ext/vector19.C
===================================================================
--- testsuite/g++.dg/ext/vector19.C	(revision 200742)
+++ testsuite/g++.dg/ext/vector19.C	(working copy)
@@ -30,22 +30,21 @@  void i2 (vec2 *x, vec2 *y, vec2u *z)
 {
   *x = *y ? *x : *y;
   *y = *z ? *x : *y;
 }
 
 void j (vec2 *x, vec2 *y, vec2 *z, vec *t)
 {
   *x = (*y < *z) ? *x : 4.2; /* { dg-error "" } */
   *y = (*x < *z) ? 2.5 : *y; /* { dg-error "" } */
   *t = *t ? *t : *t; /* { dg-error "" } */
-  *z = (*x < *z) ? '1' : '0'; /* { dg-error "" } */
-  // The last one may eventually be accepted.
+  *z = (*x < *z) ? '1' : '0';
 }
 
 template <class A, class B>
 auto k (A *a, B b) -> decltype (*a ? *a : b);
 
 void k (...) {}
 
 void l (vec2 *v, double x)
 {
   k (v, x);
Index: testsuite/g++.dg/ext/vector23.C
===================================================================
--- testsuite/g++.dg/ext/vector23.C	(revision 0)
+++ testsuite/g++.dg/ext/vector23.C	(revision 0)
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu++1y" } */
+
+typedef long vecl __attribute__((vector_size(4*sizeof(long))));
+typedef short vecs __attribute__((vector_size(8*sizeof(short))));
+typedef char vecc __attribute__((vector_size(16*sizeof(char))));
+
+auto f(vecl*a,float d,char i){
+  return (*a<0)?d:i;
+}
+auto g(vecs*a){
+  return (*a<0)?3LL:42UL;
+}
+auto h(vecc*a){
+  return (*a<0)?1:0.; // { dg-error "could not find a floating point type" }
+}

Property changes on: testsuite/g++.dg/ext/vector23.C
___________________________________________________________________
Added: svn:eol-style
   + native
Added: svn:keywords
   + Author Date Id Revision URL

Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 200742)
+++ cp/typeck.c	(working copy)
@@ -4528,37 +4528,52 @@  cp_build_binary_op (location_t location,
 	    warning (OPT_Waddress, "comparison with string literal results in unspecified behaviour");
 	}
 
       if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE)
 	{
 	vector_compare:
 	  tree intt;
 	  if (!same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (type0),
 							  TREE_TYPE (type1)))
 	    {
-	      error_at (location, "comparing vectors with different "
-				  "element types");
-	      inform (location, "operand types are %qT and %qT", type0, type1);
+	      if (complain & tf_error)
+		{
+		  error_at (location, "comparing vectors with different "
+				      "element types");
+		  inform (location, "operand types are %qT and %qT",
+			  type0, type1);
+		}
 	      return error_mark_node;
 	    }
 
 	  if (TYPE_VECTOR_SUBPARTS (type0) != TYPE_VECTOR_SUBPARTS (type1))
 	    {
-	      error_at (location, "comparing vectors with different "
-				  "number of elements");
-	      inform (location, "operand types are %qT and %qT", type0, type1);
+	      if (complain & tf_error)
+		{
+		  error_at (location, "comparing vectors with different "
+				      "number of elements");
+		  inform (location, "operand types are %qT and %qT",
+			  type0, type1);
+		}
 	      return error_mark_node;
 	    }
 
 	  /* Always construct signed integer vector type.  */
 	  intt = c_common_type_for_size (GET_MODE_BITSIZE
 					   (TYPE_MODE (TREE_TYPE (type0))), 0);
+	  if (!intt)
+	    {
+	      if (complain & tf_error)
+		error_at (location, "could not find an integer type "
+			  "of the same size as %qT", TREE_TYPE (type0));
+	      return error_mark_node;
+	    }
 	  result_type = build_opaque_vector_type (intt,
 						  TYPE_VECTOR_SUBPARTS (type0));
 	  converted = 1;
 	  break;
 	}
       build_type = boolean_type_node;
       if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE
 	   || code0 == ENUMERAL_TYPE)
 	   && (code1 == INTEGER_TYPE || code1 == REAL_TYPE
 	       || code1 == ENUMERAL_TYPE))
Index: cp/call.c
===================================================================
--- cp/call.c	(revision 200742)
+++ cp/call.c	(working copy)
@@ -4398,46 +4398,115 @@  build_conditional_expr_1 (location_t loc
       arg2 = force_rvalue (arg2, complain);
       arg3 = force_rvalue (arg3, complain);
 
       tree arg1_type = TREE_TYPE (arg1);
       arg2_type = TREE_TYPE (arg2);
       arg3_type = TREE_TYPE (arg3);
 
       if (TREE_CODE (arg2_type) != VECTOR_TYPE
 	  && TREE_CODE (arg3_type) != VECTOR_TYPE)
 	{
-	  if (complain & tf_error)
-	    error_at (loc, "at least one operand of a vector conditional "
-		      "operator must be a vector");
-	  return error_mark_node;
+	  /* Rely on the error messages of the scalar version.  */
+	  tree scal = build_conditional_expr_1 (loc, integer_one_node,
+						orig_arg2, orig_arg3, complain);
+	  if (scal == error_mark_node)
+	    return error_mark_node;
+	  tree stype = TREE_TYPE (scal);
+	  tree ctype = TREE_TYPE (arg1_type);
+	  if (TYPE_SIZE (stype) == TYPE_SIZE (ctype)
+	      && (INTEGRAL_TYPE_P (stype) || SCALAR_FLOAT_TYPE_P (stype)))
+	    ;
+	      /* Guess a type of the right size.  */
+	  else if (INTEGRAL_TYPE_P (stype))
+	    {
+	      stype = c_common_type_for_size (GET_MODE_BITSIZE
+		  (TYPE_MODE (ctype)), TYPE_UNSIGNED (stype));
+	      if (!stype)
+		{
+		  if (complain & tf_error)
+		    error_at (loc, "could not find an integer type "
+				   "of the same size as %qT", ctype);
+		  return error_mark_node;
+		}
+	    }
+	  else if (SCALAR_FLOAT_TYPE_P (stype))
+	    {
+	      if (TYPE_SIZE (ctype) == TYPE_SIZE (float_type_node))
+		stype = float_type_node;
+	      else if (TYPE_SIZE (ctype) == TYPE_SIZE (double_type_node))
+		stype = double_type_node;
+	      else if (TYPE_SIZE (ctype) == TYPE_SIZE (long_double_type_node))
+		stype = long_double_type_node;
+	      else
+		{
+		  if (complain & tf_error)
+		    error_at (loc, "could not find a floating point type "
+				   "of the same size as %qT", ctype);
+		  return error_mark_node;
+		}
+	    }
+	  else
+	    {
+	      if (complain & tf_error)
+		error_at (loc, "guessing a type for conditional expressions is "
+			  "only implemented for integers and floating point");
+	      return error_mark_node;
+	    }
+	  tree vtype = build_opaque_vector_type (stype,
+			 TYPE_VECTOR_SUBPARTS (arg1_type));
+	  if (unsafe_conversion_p (stype, arg2, complain & tf_warning))
+	    {
+	      if (complain & tf_error)
+		error_at (loc, "conversion of scalar %qT to vector %qT "
+			       "involves truncation", arg2_type, vtype);
+	      return error_mark_node;
+	    }
+	  if (unsafe_conversion_p (stype, arg3, complain & tf_warning))
+	    {
+	      if (complain & tf_error)
+		error_at (loc, "conversion of scalar %qT to vector %qT "
+			       "involves truncation", arg3_type, vtype);
+	      return error_mark_node;
+	    }
+
+	  arg2 = cp_convert (stype, arg2, complain);
+	  arg2 = save_expr (arg2);
+	  arg2 = build_vector_from_val (vtype, arg2);
+	  arg2_type = vtype;
+	  arg3 = cp_convert (stype, arg3, complain);
+	  arg3 = save_expr (arg3);
+	  arg3 = build_vector_from_val (vtype, arg3);
+	  arg3_type = vtype;
 	}
 
       if ((TREE_CODE (arg2_type) == VECTOR_TYPE)
 	  != (TREE_CODE (arg3_type) == VECTOR_TYPE))
 	{
 	  enum stv_conv convert_flag =
 	    scalar_to_vector (loc, VEC_COND_EXPR, arg2, arg3,
 			      complain & tf_error);
 
 	  switch (convert_flag)
 	    {
 	      case stv_error:
 		return error_mark_node;
 	      case stv_firstarg:
 		{
+		  arg2 = save_expr (arg2);
 		  arg2 = convert (TREE_TYPE (arg3_type), arg2);
 		  arg2 = build_vector_from_val (arg3_type, arg2);
 		  arg2_type = TREE_TYPE (arg2);
 		  break;
 		}
 	      case stv_secondarg:
 		{
+		  arg3 = save_expr (arg3);
 		  arg3 = convert (TREE_TYPE (arg2_type), arg3);
 		  arg3 = build_vector_from_val (arg2_type, arg3);
 		  arg3_type = TREE_TYPE (arg3);
 		  break;
 		}
 	      default:
 		break;
 	    }
 	}