Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Richard Biener]

On Wed, Mar 2, 2016 at 1:10 PM, Jakub Jelinek  wrote:
> On Wed, Mar 02, 2016 at 12:43:07PM +0100, Richard Biener wrote:
>> On Thu, Jan 28, 2016 at 8:33 PM, Marek Polacek  wrote:
>> > On Thu, Jan 28, 2016 at 04:47:04AM -0800, H.J. Lu wrote:
>> >> I got
>> >>
>> >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
>> >> warnings, line 17)
>> >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
>> >> warnings, line 18)
>> >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
>> >> warnings, line 34)
>> >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
>> >> warnings, line 35)
>> >
>> > Ah, patch(1) bogosity, fixed now.  Sorry about that.
>>
>> Note on i?86 I see (or x86_64 with -m32 -march=i586)
>>
>> /space/rguenther/src/svn/gcc-5-branch/gcc/testsuite/c-c++-common/vector-compare-4.c:29:1:
>> warning: SSE vector return without SSE enabled changes the ABI
>> [-Wpsabi]^M
>>
>> and thus the testcase fails (it has been adjusted for a similar kind
>> of error on ppc
>> on trunk but not yet on the branch(es)).
>
> Thus should fix that (your new PR70022 test fails similarly, but here we can
> use -w, on vector-compare-4.c which expects other warnings we need the prune
> for powerpc; though, as has been said earlier, it would be better if
> rs6000 started honoring -Wno-psabi and emitted inform rather than warning).
> Ok for trunk?

Ok for trunk and branches.

Richard.

> 2016-03-02  Jakub Jelinek  
>
> PR c/68062
> * c-c++-common/vector-compare-4.c: Add -Wno-psabi to dg-options.
>
> PR middle-end/70022
> * gcc.dg/pr70022.c: Add -w -Wno-psabi to dg-options.
>
> --- gcc/testsuite/c-c++-common/vector-compare-4.c.jj2016-02-03 
> 23:36:38.0 +0100
> +++ gcc/testsuite/c-c++-common/vector-compare-4.c   2016-03-02 
> 13:07:29.229073904 +0100
> @@ -1,6 +1,6 @@
>  /* PR c/68062 */
>  /* { dg-do compile } */
> -/* { dg-options "-Wsign-compare" } */
> +/* { dg-options "-Wsign-compare -Wno-psabi" } */
>  /* Ignore warning on some powerpc configurations. */
>  /* { dg-prune-output "non-standard ABI extension" } */
>
> --- gcc/testsuite/gcc.dg/pr70022.c.jj   2016-03-01 19:23:49.0 +0100
> +++ gcc/testsuite/gcc.dg/pr70022.c  2016-03-02 13:04:06.531860784 +0100
> @@ -1,4 +1,5 @@
>  /* { dg-do compile } */
> +/* { dg-options "-w -Wno-psabi" } */
>
>  typedef int v4si __attribute__ ((vector_size (16)));
>
>
>
> Jakub

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Marek Polacek]

On Wed, Mar 02, 2016 at 01:10:51PM +0100, Jakub Jelinek wrote:
> On Wed, Mar 02, 2016 at 12:43:07PM +0100, Richard Biener wrote:
> > On Thu, Jan 28, 2016 at 8:33 PM, Marek Polacek  wrote:
> > > On Thu, Jan 28, 2016 at 04:47:04AM -0800, H.J. Lu wrote:
> > >> I got
> > >>
> > >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> > >> warnings, line 17)
> > >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> > >> warnings, line 18)
> > >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> > >> warnings, line 34)
> > >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> > >> warnings, line 35)
> > >
> > > Ah, patch(1) bogosity, fixed now.  Sorry about that.
> > 
> > Note on i?86 I see (or x86_64 with -m32 -march=i586)
> > 
> > /space/rguenther/src/svn/gcc-5-branch/gcc/testsuite/c-c++-common/vector-compare-4.c:29:1:
> > warning: SSE vector return without SSE enabled changes the ABI
> > [-Wpsabi]^M
> > 
> > and thus the testcase fails (it has been adjusted for a similar kind
> > of error on ppc
> > on trunk but not yet on the branch(es)).
> 
> Thus should fix that (your new PR70022 test fails similarly, but here we can
> use -w, on vector-compare-4.c which expects other warnings we need the prune
> for powerpc; though, as has been said earlier, it would be better if
> rs6000 started honoring -Wno-psabi and emitted inform rather than warning).
> Ok for trunk?
> 
> 2016-03-02  Jakub Jelinek  
> 
>   PR c/68062
>   * c-c++-common/vector-compare-4.c: Add -Wno-psabi to dg-options.
> 
>   PR middle-end/70022
>   * gcc.dg/pr70022.c: Add -w -Wno-psabi to dg-options.
> 
> --- gcc/testsuite/c-c++-common/vector-compare-4.c.jj  2016-02-03 
> 23:36:38.0 +0100
> +++ gcc/testsuite/c-c++-common/vector-compare-4.c 2016-03-02 
> 13:07:29.229073904 +0100
> @@ -1,6 +1,6 @@
>  /* PR c/68062 */
>  /* { dg-do compile } */
> -/* { dg-options "-Wsign-compare" } */
> +/* { dg-options "-Wsign-compare -Wno-psabi" } */
>  /* Ignore warning on some powerpc configurations. */
>  /* { dg-prune-output "non-standard ABI extension" } */

Can't approve, but I'm certainly fine with it.  Thanks for fixing.

Marek

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Jakub Jelinek]

On Wed, Mar 02, 2016 at 12:43:07PM +0100, Richard Biener wrote:
> On Thu, Jan 28, 2016 at 8:33 PM, Marek Polacek  wrote:
> > On Thu, Jan 28, 2016 at 04:47:04AM -0800, H.J. Lu wrote:
> >> I got
> >>
> >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> >> warnings, line 17)
> >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> >> warnings, line 18)
> >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> >> warnings, line 34)
> >> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> >> warnings, line 35)
> >
> > Ah, patch(1) bogosity, fixed now.  Sorry about that.
> 
> Note on i?86 I see (or x86_64 with -m32 -march=i586)
> 
> /space/rguenther/src/svn/gcc-5-branch/gcc/testsuite/c-c++-common/vector-compare-4.c:29:1:
> warning: SSE vector return without SSE enabled changes the ABI
> [-Wpsabi]^M
> 
> and thus the testcase fails (it has been adjusted for a similar kind
> of error on ppc
> on trunk but not yet on the branch(es)).

Thus should fix that (your new PR70022 test fails similarly, but here we can
use -w, on vector-compare-4.c which expects other warnings we need the prune
for powerpc; though, as has been said earlier, it would be better if
rs6000 started honoring -Wno-psabi and emitted inform rather than warning).
Ok for trunk?

2016-03-02  Jakub Jelinek  

PR c/68062
* c-c++-common/vector-compare-4.c: Add -Wno-psabi to dg-options.

PR middle-end/70022
* gcc.dg/pr70022.c: Add -w -Wno-psabi to dg-options.

--- gcc/testsuite/c-c++-common/vector-compare-4.c.jj2016-02-03 
23:36:38.0 +0100
+++ gcc/testsuite/c-c++-common/vector-compare-4.c   2016-03-02 
13:07:29.229073904 +0100
@@ -1,6 +1,6 @@
 /* PR c/68062 */
 /* { dg-do compile } */
-/* { dg-options "-Wsign-compare" } */
+/* { dg-options "-Wsign-compare -Wno-psabi" } */
 /* Ignore warning on some powerpc configurations. */
 /* { dg-prune-output "non-standard ABI extension" } */
 
--- gcc/testsuite/gcc.dg/pr70022.c.jj   2016-03-01 19:23:49.0 +0100
+++ gcc/testsuite/gcc.dg/pr70022.c  2016-03-02 13:04:06.531860784 +0100
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-options "-w -Wno-psabi" } */
 
 typedef int v4si __attribute__ ((vector_size (16)));
 


Jakub

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Richard Biener]

On Thu, Jan 28, 2016 at 8:33 PM, Marek Polacek  wrote:
> On Thu, Jan 28, 2016 at 04:47:04AM -0800, H.J. Lu wrote:
>> I got
>>
>> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
>> warnings, line 17)
>> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
>> warnings, line 18)
>> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
>> warnings, line 34)
>> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
>> warnings, line 35)
>
> Ah, patch(1) bogosity, fixed now.  Sorry about that.

Note on i?86 I see (or x86_64 with -m32 -march=i586)

/space/rguenther/src/svn/gcc-5-branch/gcc/testsuite/c-c++-common/vector-compare-4.c:29:1:
warning: SSE vector return without SSE enabled changes the ABI
[-Wpsabi]^M

and thus the testcase fails (it has been adjusted for a similar kind
of error on ppc
on trunk but not yet on the branch(es)).

Richard.

> Marek

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Marek Polacek]

On Thu, Jan 28, 2016 at 04:47:04AM -0800, H.J. Lu wrote:
> I got
> 
> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> warnings, line 17)
> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> warnings, line 18)
> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> warnings, line 34)
> FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
> warnings, line 35)

Ah, patch(1) bogosity, fixed now.  Sorry about that.

Marek

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[H.J. Lu]

On Wed, Jan 27, 2016 at 9:45 AM, Marek Polacek  wrote:
> On Wed, Jan 27, 2016 at 10:02:36AM -0700, Jeff Law wrote:
>> On 01/27/2016 12:26 AM, Marek Polacek wrote:
>> >Ping.
>> >
>> >On Wed, Jan 20, 2016 at 12:31:51PM +0100, Marek Polacek wrote:
>> >>On Wed, Jan 13, 2016 at 11:11:52PM +, Joseph Myers wrote:
>> >>>The C front-end changes are OK.
>> >>
>> >>Jason, is the C++ part of this patch here
>> >>
>> >>(which is identical to the change in the C FE) ok?
>> >>
>> >>Also, not sure about backporting this, maybe just to 5?
>> I'll go ahead and ack the C++ bits.  This is fine for the trunk.
>
> Thanks.
>
>> WRT backporting, your call.
>
> I think I'll put it into GCC 5 (it's safe and should apply cleanly),
> but leave 4.9 alone.
>
> Marek

I got

FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
warnings, line 17)
FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
warnings, line 18)
FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
warnings, line 34)
FAIL: c-c++-common/vector-compare-4.c  -Wc++-compat   (test for
warnings, line 35)

on x86 on GCC 5 branch.

-- 
H.J.

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Jeff Law]

On 01/27/2016 12:26 AM, Marek Polacek wrote:

Ping.

On Wed, Jan 20, 2016 at 12:31:51PM +0100, Marek Polacek wrote:

On Wed, Jan 13, 2016 at 11:11:52PM +, Joseph Myers wrote:

The C front-end changes are OK.


Jason, is the C++ part of this patch here

(which is identical to the change in the C FE) ok?

Also, not sure about backporting this, maybe just to 5?

I'll go ahead and ack the C++ bits.  This is fine for the trunk.

WRT backporting, your call.

jeff

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Marek Polacek]

On Wed, Jan 27, 2016 at 10:02:36AM -0700, Jeff Law wrote:
> On 01/27/2016 12:26 AM, Marek Polacek wrote:
> >Ping.
> >
> >On Wed, Jan 20, 2016 at 12:31:51PM +0100, Marek Polacek wrote:
> >>On Wed, Jan 13, 2016 at 11:11:52PM +, Joseph Myers wrote:
> >>>The C front-end changes are OK.
> >>
> >>Jason, is the C++ part of this patch here
> >>
> >>(which is identical to the change in the C FE) ok?
> >>
> >>Also, not sure about backporting this, maybe just to 5?
> I'll go ahead and ack the C++ bits.  This is fine for the trunk.

Thanks.
 
> WRT backporting, your call.

I think I'll put it into GCC 5 (it's safe and should apply cleanly),
but leave 4.9 alone.

Marek

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Marek Polacek]

Ping.

On Wed, Jan 20, 2016 at 12:31:51PM +0100, Marek Polacek wrote:
> On Wed, Jan 13, 2016 at 11:11:52PM +, Joseph Myers wrote:
> > The C front-end changes are OK.
> 
> Jason, is the C++ part of this patch here
> 
> (which is identical to the change in the C FE) ok?
> 
> Also, not sure about backporting this, maybe just to 5?

Marek

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Marek Polacek]

On Wed, Jan 13, 2016 at 11:11:52PM +, Joseph Myers wrote:
> The C front-end changes are OK.

Jason, is the C++ part of this patch here

(which is identical to the change in the C FE) ok?

Also, not sure about backporting this, maybe just to 5?

Marek

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Marek Polacek]

On Wed, Jan 13, 2016 at 06:53:06PM +, Joseph Myers wrote:
> Will -Wsign-compare warnings be generated for the implicit signed / 
> unsigned conversions in comparisons, as for scalar comparisons?

Good point.  No, with the previous patch -Wsign-compare would be quiet.  But
since it probably should warn, I've added the warning (warn_for_sign_compare
isn't prepared to handle vectors so I've just used warning_at).

Regtested on x86_64-linux, bootstrap in progress, but I don't expect any
problems.

Ok for trunk?  What about branches, is this suitable for 5?

2016-01-13  Marek Polacek  

PR c/68062
* c-typeck.c (build_binary_op) [EQ_EXPR, GE_EXPR]: Promote operand
to unsigned, if needed.  Add -Wsign-compare warning.

* typeck.c (cp_build_binary_op): Promote operand to unsigned, if
needed.  Add -Wsign-compare warning.

* c-c++-common/vector-compare-4.c: New test.

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 952041b..0fe1d46 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -11048,6 +11048,20 @@ build_binary_op (location_t location, enum tree_code 
code,
   return error_mark_node;
 }
 
+ /* It's not precisely specified how the usual arithmetic
+conversions apply to the vector types.  Here, we use
+the unsigned type if one of the operands is signed and
+the other one is unsigned.  */
+ if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
+   {
+ if (!TYPE_UNSIGNED (type0))
+   op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
+ else
+   op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
+ warning_at (location, OPT_Wsign_compare, "comparison between "
+ "types %qT and %qT", type0, type1);
+   }
+
   /* Always construct signed integer vector type.  */
   intt = c_common_type_for_size (GET_MODE_BITSIZE
   (TYPE_MODE (TREE_TYPE (type0))), 0);
@@ -11201,6 +11215,20 @@ build_binary_op (location_t location, enum tree_code 
code,
   return error_mark_node;
 }
 
+ /* It's not precisely specified how the usual arithmetic
+conversions apply to the vector types.  Here, we use
+the unsigned type if one of the operands is signed and
+the other one is unsigned.  */
+ if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
+   {
+ if (!TYPE_UNSIGNED (type0))
+   op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
+ else
+   op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
+ warning_at (location, OPT_Wsign_compare, "comparison between "
+ "types %qT and %qT", type0, type1);
+   }
+
   /* Always construct signed integer vector type.  */
   intt = c_common_type_for_size (GET_MODE_BITSIZE
   (TYPE_MODE (TREE_TYPE (type0))), 0);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 472b41b..2f0035a 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -4813,6 +4813,20 @@ cp_build_binary_op (location_t location,
  return error_mark_node;
}
 
+ /* It's not precisely specified how the usual arithmetic
+conversions apply to the vector types.  Here, we use
+the unsigned type if one of the operands is signed and
+the other one is unsigned.  */
+ if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
+   {
+ if (!TYPE_UNSIGNED (type0))
+   op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
+ else
+   op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
+ warning_at (location, OPT_Wsign_compare, "comparison between "
+ "types %qT and %qT", type0, type1);
+   }
+
  /* Always construct signed integer vector type.  */
  intt = c_common_type_for_size (GET_MODE_BITSIZE
   (TYPE_MODE (TREE_TYPE (type0))), 0);
diff --git gcc/testsuite/c-c++-common/vector-compare-4.c 
gcc/testsuite/c-c++-common/vector-compare-4.c
index e69de29..b44f474 100644
--- gcc/testsuite/c-c++-common/vector-compare-4.c
+++ gcc/testsuite/c-c++-common/vector-compare-4.c
@@ -0,0 +1,42 @@
+/* PR c/68062 */
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare" } */
+
+typedef signed char __attribute__ ((vector_size (4))) v4qi;
+typedef unsigned char __attribute__ ((vector_size (4))) uv4qi;
+typedef signed int __attribute__ ((vector_size (4 * __SIZEOF_INT__))) v4si;
+typedef unsigned int __attribute__ ((vector_size (4 * __SIZEOF_INT__))) uv4si;
+
+v4qi
+fn1 (void)
+{
+  v4qi a = { 1, 2, 3, 4 };
+  uv4qi b = { 4, 3, 2, 1 };
+  v4qi v = { 0, 0, 0, 0 };
+
+  v += (a == b); /* { dg-warning "comparison between types" } */
+  v += (a != b); /* { dg-warning "comparison 

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Richard Biener]

On January 13, 2016 4:26:50 PM GMT+01:00, Marek Polacek  
wrote:
>We crash on the following testcase because the FEs create 
>
>  VEC_COND_EXPR < a == b , { -1, -1, -1, -1 } , { 0, 0, 0, 0 } >
>
>where the operands of the comparison are same except for the sign (it's
>vector_types_compatible_elements_p that says that).  But GIMPLE doesn't
>like the difference in the sign.
>
>As I read the discussion in the PR, the way forward here is to perform
>signed -> unsigned conversions.  So that is what I do in the following.
>
>Bootstrapped/regtested on x86_64-linux and ppc64le-linux, ok for trunk?

Looks good from a middle-end perspective.

Richard.

>2016-01-13  Marek Polacek  
>
>   PR c/68062
>   * c-typeck.c (build_binary_op) [EQ_EXPR, GE_EXPR]: Promote operand
>   to unsigned, if needed.
>
>   * typeck.c (cp_build_binary_op): Promote operand to unsigned, if
>   needed.
>
>   * c-c++-common/vector-compare-4.c: New test.
>
>diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
>index 952041b..b646451 100644
>--- gcc/c/c-typeck.c
>+++ gcc/c/c-typeck.c
>@@ -11048,6 +11048,18 @@ build_binary_op (location_t location, enum
>tree_code code,
>   return error_mark_node;
> }
> 
>+/* It's not precisely specified how the usual arithmetic
>+   conversions apply to the vector types.  Here, we use
>+   the unsigned type if one of the operands is signed and
>+   the other one is unsigned.  */
>+if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
>+  {
>+if (!TYPE_UNSIGNED (type0))
>+  op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
>+else
>+  op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
>+  }
>+
>   /* Always construct signed integer vector type.  */
>   intt = c_common_type_for_size (GET_MODE_BITSIZE
>  (TYPE_MODE (TREE_TYPE (type0))), 0);
>@@ -11201,6 +11213,18 @@ build_binary_op (location_t location, enum
>tree_code code,
>   return error_mark_node;
> }
> 
>+/* It's not precisely specified how the usual arithmetic
>+   conversions apply to the vector types.  Here, we use
>+   the unsigned type if one of the operands is signed and
>+   the other one is unsigned.  */
>+if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
>+  {
>+if (!TYPE_UNSIGNED (type0))
>+  op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
>+else
>+  op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
>+  }
>+
>   /* Always construct signed integer vector type.  */
>   intt = c_common_type_for_size (GET_MODE_BITSIZE
>  (TYPE_MODE (TREE_TYPE (type0))), 0);
>diff --git gcc/cp/typeck.c gcc/cp/typeck.c
>index 472b41b..ffa9ed4 100644
>--- gcc/cp/typeck.c
>+++ gcc/cp/typeck.c
>@@ -4813,6 +4813,18 @@ cp_build_binary_op (location_t location,
> return error_mark_node;
>   }
> 
>+/* It's not precisely specified how the usual arithmetic
>+   conversions apply to the vector types.  Here, we use
>+   the unsigned type if one of the operands is signed and
>+   the other one is unsigned.  */
>+if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
>+  {
>+if (!TYPE_UNSIGNED (type0))
>+  op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
>+else
>+  op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
>+  }
>+
> /* Always construct signed integer vector type.  */
> intt = c_common_type_for_size (GET_MODE_BITSIZE
>  (TYPE_MODE (TREE_TYPE (type0))), 0);
>diff --git gcc/testsuite/c-c++-common/vector-compare-4.c
>gcc/testsuite/c-c++-common/vector-compare-4.c
>index e69de29..14faf04 100644
>--- gcc/testsuite/c-c++-common/vector-compare-4.c
>+++ gcc/testsuite/c-c++-common/vector-compare-4.c
>@@ -0,0 +1,41 @@
>+/* PR c/68062 */
>+/* { dg-do compile } */
>+
>+typedef signed char __attribute__ ((vector_size (4))) v4qi;
>+typedef unsigned char __attribute__ ((vector_size (4))) uv4qi;
>+typedef signed int __attribute__ ((vector_size (4 * __SIZEOF_INT__)))
>v4si;
>+typedef unsigned int __attribute__ ((vector_size (4 *
>__SIZEOF_INT__))) uv4si;
>+
>+v4qi
>+fn1 (void)
>+{
>+  v4qi a = { 1, 2, 3, 4 };
>+  uv4qi b = { 4, 3, 2, 1 };
>+  v4qi v = { 0, 0, 0, 0 };
>+
>+  v += (a == b);
>+  v += (a != b);
>+  v += (a >= b);
>+  v += (a <= b);
>+  v += (a > b);
>+  v += (a < b);
>+
>+  return v;
>+}
>+
>+v4si
>+fn2 (void)
>+{
>+  v4si a = { 1, 2, 3, 4 };
>+  uv4si b = { 4, 3, 2, 1 };
>+  v4si v = { 0, 0, 0, 0 };
>+
>+  v += (a == b);
>+  v += (a != b);
>+  v += (a >= b);
>+  v += (a <= b);
>+  v += (a > b);
>+  v += (a < b);
>+
>+  return v;
>+}
>
>   Marek

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Joseph Myers]

Will -Wsign-compare warnings be generated for the implicit signed / 
unsigned conversions in comparisons, as for scalar comparisons?

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Joseph Myers]

On Wed, 13 Jan 2016, Marek Polacek wrote:

> On Wed, Jan 13, 2016 at 06:53:06PM +, Joseph Myers wrote:
> > Will -Wsign-compare warnings be generated for the implicit signed / 
> > unsigned conversions in comparisons, as for scalar comparisons?
> 
> Good point.  No, with the previous patch -Wsign-compare would be quiet.  But
> since it probably should warn, I've added the warning (warn_for_sign_compare
> isn't prepared to handle vectors so I've just used warning_at).
> 
> Regtested on x86_64-linux, bootstrap in progress, but I don't expect any
> problems.

The C front-end changes are OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

[C/C++ PATCH] Don't emit invalid VEC_COND_EXPR for vector comparisons (PR c/68062)

[Marek Polacek]

We crash on the following testcase because the FEs create 

  VEC_COND_EXPR < a == b , { -1, -1, -1, -1 } , { 0, 0, 0, 0 } >

where the operands of the comparison are same except for the sign (it's
vector_types_compatible_elements_p that says that).  But GIMPLE doesn't
like the difference in the sign.

As I read the discussion in the PR, the way forward here is to perform
signed -> unsigned conversions.  So that is what I do in the following.

Bootstrapped/regtested on x86_64-linux and ppc64le-linux, ok for trunk?

2016-01-13  Marek Polacek  

PR c/68062
* c-typeck.c (build_binary_op) [EQ_EXPR, GE_EXPR]: Promote operand
to unsigned, if needed.

* typeck.c (cp_build_binary_op): Promote operand to unsigned, if
needed.

* c-c++-common/vector-compare-4.c: New test.

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 952041b..b646451 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -11048,6 +11048,18 @@ build_binary_op (location_t location, enum tree_code 
code,
   return error_mark_node;
 }
 
+ /* It's not precisely specified how the usual arithmetic
+conversions apply to the vector types.  Here, we use
+the unsigned type if one of the operands is signed and
+the other one is unsigned.  */
+ if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
+   {
+ if (!TYPE_UNSIGNED (type0))
+   op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
+ else
+   op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
+   }
+
   /* Always construct signed integer vector type.  */
   intt = c_common_type_for_size (GET_MODE_BITSIZE
   (TYPE_MODE (TREE_TYPE (type0))), 0);
@@ -11201,6 +11213,18 @@ build_binary_op (location_t location, enum tree_code 
code,
   return error_mark_node;
 }
 
+ /* It's not precisely specified how the usual arithmetic
+conversions apply to the vector types.  Here, we use
+the unsigned type if one of the operands is signed and
+the other one is unsigned.  */
+ if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
+   {
+ if (!TYPE_UNSIGNED (type0))
+   op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
+ else
+   op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
+   }
+
   /* Always construct signed integer vector type.  */
   intt = c_common_type_for_size (GET_MODE_BITSIZE
   (TYPE_MODE (TREE_TYPE (type0))), 0);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 472b41b..ffa9ed4 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -4813,6 +4813,18 @@ cp_build_binary_op (location_t location,
  return error_mark_node;
}
 
+ /* It's not precisely specified how the usual arithmetic
+conversions apply to the vector types.  Here, we use
+the unsigned type if one of the operands is signed and
+the other one is unsigned.  */
+ if (TYPE_UNSIGNED (type0) != TYPE_UNSIGNED (type1))
+   {
+ if (!TYPE_UNSIGNED (type0))
+   op0 = build1 (VIEW_CONVERT_EXPR, type1, op0);
+ else
+   op1 = build1 (VIEW_CONVERT_EXPR, type0, op1);
+   }
+
  /* Always construct signed integer vector type.  */
  intt = c_common_type_for_size (GET_MODE_BITSIZE
   (TYPE_MODE (TREE_TYPE (type0))), 0);
diff --git gcc/testsuite/c-c++-common/vector-compare-4.c 
gcc/testsuite/c-c++-common/vector-compare-4.c
index e69de29..14faf04 100644
--- gcc/testsuite/c-c++-common/vector-compare-4.c
+++ gcc/testsuite/c-c++-common/vector-compare-4.c
@@ -0,0 +1,41 @@
+/* PR c/68062 */
+/* { dg-do compile } */
+
+typedef signed char __attribute__ ((vector_size (4))) v4qi;
+typedef unsigned char __attribute__ ((vector_size (4))) uv4qi;
+typedef signed int __attribute__ ((vector_size (4 * __SIZEOF_INT__))) v4si;
+typedef unsigned int __attribute__ ((vector_size (4 * __SIZEOF_INT__))) uv4si;
+
+v4qi
+fn1 (void)
+{
+  v4qi a = { 1, 2, 3, 4 };
+  uv4qi b = { 4, 3, 2, 1 };
+  v4qi v = { 0, 0, 0, 0 };
+
+  v += (a == b);
+  v += (a != b);
+  v += (a >= b);
+  v += (a <= b);
+  v += (a > b);
+  v += (a < b);
+
+  return v;
+}
+
+v4si
+fn2 (void)
+{
+  v4si a = { 1, 2, 3, 4 };
+  uv4si b = { 4, 3, 2, 1 };
+  v4si v = { 0, 0, 0, 0 };
+
+  v += (a == b);
+  v += (a != b);
+  v += (a >= b);
+  v += (a <= b);
+  v += (a > b);
+  v += (a < b);
+
+  return v;
+}

Marek