diff mbox

RFA: Consider int and same-size short as equivalent vector components

Message ID 20130827182953.861lvln5p0ckggkg-nzlynne@webmail.spamcop.net
State New
Headers show

Commit Message

Joern Rennecke Aug. 27, 2013, 10:29 p.m. UTC 
Quoting Marc Glisse <marc.glisse@inria.fr>:

> On Mon, 26 Aug 2013, Joern Rennecke wrote:
>
>> Quoting Marc Glisse <marc.glisse@inria.fr>:
>>
>>> The issue seems larger than just short/int. On x86, (l<l)<l fails to
>>> compile for a vector of long, with l<l that has opaque type vector of
>>> int, that seems wrong.
>>
>> I don't understand what you mean here.  Could you post the actual   
>> code sample?
>
> typedef long vl __attribute__((vector_size(16)));
>
> void f(vl l){
>   (l<l)<l;
> }
>
> it compiles fine on x86_64 but not on x86.
> (btw, my sentence was ambiguous, what I find wrong is the failure to
> compile, not the type of l<l)

Checking what tree.h has to say about the meaning of TYPE_VECTOR_OPAQUE,
I found it says:
/* Nonzero in a VECTOR_TYPE if the frontends should not emit warnings
    about missing conversions to other vector types of the same size.  */

So, presumably, the front ends should accept any combination of an opaque
vector with another vector (opaque or not) if the operation can be performed.
I don't want into all the ways a conversion might be used to make an
operation work at this time, but at least we should accept combinations
of vectors that are essentially the same.

I found for our test cases, this not only requires replacing the two
extant uses of same_scalar_type_ignoring_signedness, but also
replacing/augmenting three euqality comparisons.

For the C frontend, these were two simple equality comparisons, which
could be replaced straightforwardly;  For the C++ frontend, I'm not sure
if the same_type_ignoring_top_level_qualifiers_p check becomes redundant,
and less so, if so, if it'll stay redundant in the future.
The safe option seemed to be to have both tests.  I added the C++ front end
to the CC list in case they have some insight / opinion to offer on this.

I have successfully bootstrapped / regtested the attached patch on
i686-pc-linux-gnu.
2013-08-27  Joern Rennecke  <joern.rennecke@embecosm.com>

gcc/c-family:
	* c-common.c (same_scalar_type_ignoring_signedness): Delete.
	(vector_types_compatible_elements_p): New function.
	* c-common.h: (same_scalar_type_ignoring_signedness): Delete prototype.
        (vector_types_compatible_elements_p): Prototype.
gcc/c:
	* c-typeck.c (build_binary_op): Use vector_types_compatible_elements_p.
gcc/cp:
	* typeck.c (cp_build_binary_op): Use vector_types_compatible_elements_p.
gcc/testsuite:
	* c-c++-common/opaque-vector.c: New test.

Comments

Paolo Carlini Aug. 27, 2013, 10:37 p.m. UTC | #1 
Hi,

On 08/28/2013 12:29 AM, Joern Rennecke wrote:
> 	* c-common.h: (same_scalar_type_ignoring_signedness): Delete prototype.
>          (vector_types_compatible_elements_p): Prototype.
Sorry for nitpicking, but since we are now using C++ I think declaration 
is more correct than prototype.

Paolo.
diff mbox

Patch

Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 202008)
+++ c-family/c-common.c	(working copy)
@@ -10690,20 +10690,36 @@  resolve_overloaded_builtin (location_t l
     }
 }
 
-/* Ignoring their sign, return true if two scalar types are the same.  */
+/* True if vector types T1 and T2 can be inputs to the same binary
+   operator without conversion.
+   We don't check the overall vector size here because some of our callers
+   want to give different error messages when the vectors are compatible
+   except for the element count.  */
+
 bool
-same_scalar_type_ignoring_signedness (tree t1, tree t2)
+vector_types_compatible_elements_p (tree t1, tree t2)
 {
+  bool opaque = TYPE_VECTOR_OPAQUE (t1) || TYPE_VECTOR_OPAQUE (t2);
+  t1 = TREE_TYPE (t1);
+  t2 = TREE_TYPE (t2);
+
   enum tree_code c1 = TREE_CODE (t1), c2 = TREE_CODE (t2);
 
   gcc_assert ((c1 == INTEGER_TYPE || c1 == REAL_TYPE || c1 == FIXED_POINT_TYPE)
 	      && (c2 == INTEGER_TYPE || c2 == REAL_TYPE
 		  || c2 == FIXED_POINT_TYPE));
 
+  t1 = c_common_signed_type (t1);
+  t2 = c_common_signed_type (t2);
   /* Equality works here because c_common_signed_type uses
      TYPE_MAIN_VARIANT.  */
-  return c_common_signed_type (t1)
-    == c_common_signed_type (t2);
+  if (t1 == t2)
+    return true;
+  if (opaque && c1 == c2
+      && (c1 == INTEGER_TYPE || c1 == REAL_TYPE)
+      && TYPE_PRECISION (t1) == TYPE_PRECISION (t2))
+    return true;
+  return false;
 }
 
 /* Check for missing format attributes on function pointers.  LTYPE is
Index: c-family/c-common.h
===================================================================
--- c-family/c-common.h	(revision 202008)
+++ c-family/c-common.h	(working copy)
@@ -766,7 +766,7 @@  extern void warn_logical_operator (locat
 				   enum tree_code, tree, enum tree_code, tree);
 extern void check_main_parameter_types (tree decl);
 extern bool c_determine_visibility (tree);
-extern bool same_scalar_type_ignoring_signedness (tree, tree);
+extern bool vector_types_compatible_elements_p (tree, tree);
 extern void mark_valid_location_for_stdc_pragma (bool);
 extern bool valid_location_for_stdc_pragma_p (void);
 extern void set_float_const_decimal64 (void);
Index: c/c-typeck.c
===================================================================
--- c/c-typeck.c	(revision 202008)
+++ c/c-typeck.c	(working copy)
@@ -9973,7 +9973,7 @@  build_binary_op (location_t location, en
       if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE)
         {
           tree intt;
-          if (TREE_TYPE (type0) != TREE_TYPE (type1))
+	  if (!vector_types_compatible_elements_p (type0, type1))
             {
               error_at (location, "comparing vectors with different "
                                   "element types");
@@ -10110,7 +10110,7 @@  build_binary_op (location_t location, en
       if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE)
         {
           tree intt;
-          if (TREE_TYPE (type0) != TREE_TYPE (type1))
+          if (!vector_types_compatible_elements_p (type0, type1))
             {
               error_at (location, "comparing vectors with different "
                                   "element types");
@@ -10216,8 +10216,7 @@  build_binary_op (location_t location, en
 
   if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE
       && (!tree_int_cst_equal (TYPE_SIZE (type0), TYPE_SIZE (type1))
-	  || !same_scalar_type_ignoring_signedness (TREE_TYPE (type0),
-						    TREE_TYPE (type1))))
+	  ||!vector_types_compatible_elements_p (type0, type1)))
     {
       binary_op_error (location, code, type0, type1);
       return error_mark_node;
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 202008)
+++ cp/typeck.c	(working copy)
@@ -4534,7 +4534,8 @@  cp_build_binary_op (location_t location,
 	vector_compare:
 	  tree intt;
 	  if (!same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (type0),
-							  TREE_TYPE (type1)))
+							  TREE_TYPE (type1))
+	      && !vector_types_compatible_elements_p (type0, type1))
 	    {
 	      if (complain & tf_error)
 		{
@@ -4650,8 +4651,7 @@  cp_build_binary_op (location_t location,
       if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE)
 	{
 	  if (!tree_int_cst_equal (TYPE_SIZE (type0), TYPE_SIZE (type1))
-	      || !same_scalar_type_ignoring_signedness (TREE_TYPE (type0),
-							TREE_TYPE (type1)))
+	      || !vector_types_compatible_elements_p (type0, type1))
 	    {
 	      if (complain & tf_error)
 		binary_op_error (location, code, type0, type1);
Index: testsuite/c-c++-common/opaque-vector.c
===================================================================
--- testsuite/c-c++-common/opaque-vector.c	(revision 0)
+++ testsuite/c-c++-common/opaque-vector.c	(working copy)
@@ -0,0 +1,22 @@ 
+#define B_TEST(TYPE) { TYPE v __attribute__((vector_size(16))); (void)((v < v) < v); }
+#ifdef __cplusplus
+#define T_TEST(TYPE) { TYPE s; TYPE v __attribute__((vector_size(16))); __typeof((v<v)[0]) iv __attribute__((vector_size(16))); (void)((iv ? s : s) < v); }
+#else
+#define T_TEST(TYPE)
+#endif
+#define T(TYPE) B_TEST(TYPE) T_TEST(TYPE)
+
+void f ()
+{
+  T(short)
+  T(int)
+  T(long)
+  T(long long)
+
+  T_TEST(float)
+  T_TEST(double)
+  /* Avoid trouble with non-power-of-two sizes.  */
+#if !defined(__i386__) && !defined(__x86_64__) && !defined(__m68k__) && !defined(__ia64__)
+  T_TEST(long double)
+#endif
+}