diff mbox

[rs6000] Fix PR71297 (ICE on invalid calls to vec_ld and vec_st)

Message ID 1467918661.18357.5.camel@oc8801110288.ibm.com
State New
Headers show

Commit Message

Bill Schmidt July 7, 2016, 7:11 p.m. UTC 
Hi,

PR71297 reports that we ICE when __builtin_vec_ld or __builtin_vec_st is
provided with an incorrect number of arguments.  This patch fixes it by
bypassing special handling for these intrinsics when the number of
arguments is wrong, thus allowing the standard error handling for
builtins to kick in.

The patch is pretty obvious and I think adding a test case would be
extraneous, though I can do so if desired.  Bootstrapped and tested on
powerpc64le-unknown-linux-gnu with no regressions, and the original
failure is fixed.  Is this ok for trunk?

Thanks,
Bill


2016-07-07  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/71297
	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
	Allow standard error handling to take over when a wrong number
	of arguments is presented to __builtin_vec_ld () or
	__builtin_vec_st ().

Comments

Bill Schmidt July 7, 2016, 8:40 p.m. UTC | #1 
Resending since I thinkoed Segher's email address.  Sorry for the noise.

Bill

On Thu, 2016-07-07 at 14:11 -0500, Bill Schmidt wrote:
> Hi,
> 
> PR71297 reports that we ICE when __builtin_vec_ld or __builtin_vec_st is
> provided with an incorrect number of arguments.  This patch fixes it by
> bypassing special handling for these intrinsics when the number of
> arguments is wrong, thus allowing the standard error handling for
> builtins to kick in.
> 
> The patch is pretty obvious and I think adding a test case would be
> extraneous, though I can do so if desired.  Bootstrapped and tested on
> powerpc64le-unknown-linux-gnu with no regressions, and the original
> failure is fixed.  Is this ok for trunk?
> 
> Thanks,
> Bill
> 
> 
> 2016-07-07  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> 
> 	PR target/71297
> 	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
> 	Allow standard error handling to take over when a wrong number
> 	of arguments is presented to __builtin_vec_ld () or
> 	__builtin_vec_st ().
> 
> 
> Index: gcc/config/rs6000/rs6000-c.c
> ===================================================================
> --- gcc/config/rs6000/rs6000-c.c	(revision 238120)
> +++ gcc/config/rs6000/rs6000-c.c	(working copy)
> @@ -5281,10 +5281,11 @@ assignment for unaligned loads and stores");
>       are able to honor __restrict__, for example.  We may want to
>       consider this for all memory access built-ins.
>  
> -     When -maltivec=be is specified, simply punt to existing
> -     built-in processing.  */
> +     When -maltivec=be is specified, or the wrong number of arguments
> +     is provided, simply punt to existing built-in processing.  */
>    if (fcode == ALTIVEC_BUILTIN_VEC_LD
> -      && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG))
> +      && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG)
> +      && nargs == 2)
>      {
>        tree arg0 = (*arglist)[0];
>        tree arg1 = (*arglist)[1];
> @@ -5354,7 +5355,8 @@ assignment for unaligned loads and stores");
>  
>    /* Similarly for stvx.  */
>    if (fcode == ALTIVEC_BUILTIN_VEC_ST
> -      && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG))
> +      && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG)
> +      && nargs == 3)
>      {
>        tree arg0 = (*arglist)[0];
>        tree arg1 = (*arglist)[1];
>
Segher Boessenkool July 8, 2016, 5:14 a.m. UTC | #2 
On Thu, Jul 07, 2016 at 03:40:28PM -0500, Bill Schmidt wrote:
> > PR71297 reports that we ICE when __builtin_vec_ld or __builtin_vec_st is
> > provided with an incorrect number of arguments.  This patch fixes it by
> > bypassing special handling for these intrinsics when the number of
> > arguments is wrong, thus allowing the standard error handling for
> > builtins to kick in.
> > 
> > The patch is pretty obvious and I think adding a test case would be
> > extraneous, though I can do so if desired.

Well you could use the one from the PR?

> > Bootstrapped and tested on
> > powerpc64le-unknown-linux-gnu with no regressions, and the original
> > failure is fixed.  Is this ok for trunk?

Yes, but please do a testcase.  Okay for backports, too.


Segher


> > 	PR target/71297
> > 	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
> > 	Allow standard error handling to take over when a wrong number
> > 	of arguments is presented to __builtin_vec_ld () or
> > 	__builtin_vec_st ().
Bill Schmidt July 8, 2016, 12:29 p.m. UTC | #3 
> On Jul 8, 2016, at 12:14 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> On Thu, Jul 07, 2016 at 03:40:28PM -0500, Bill Schmidt wrote:
>>> PR71297 reports that we ICE when __builtin_vec_ld or __builtin_vec_st is
>>> provided with an incorrect number of arguments.  This patch fixes it by
>>> bypassing special handling for these intrinsics when the number of
>>> arguments is wrong, thus allowing the standard error handling for
>>> builtins to kick in.
>>> 
>>> The patch is pretty obvious and I think adding a test case would be
>>> extraneous, though I can do so if desired.
> 
> Well you could use the one from the PR?
> 
>>> Bootstrapped and tested on
>>> powerpc64le-unknown-linux-gnu with no regressions, and the original
>>> failure is fixed.  Is this ok for trunk?
> 
> Yes, but please do a testcase.  Okay for backports, too.

No backports required; this is a 7 regression.

Bill

> 
> 
> Segher
> 
> 
>>> 	PR target/71297
>>> 	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
>>> 	Allow standard error handling to take over when a wrong number
>>> 	of arguments is presented to __builtin_vec_ld () or
>>> 	__builtin_vec_st ().
>
Bill Schmidt July 8, 2016, 3:47 p.m. UTC | #4 
Fixed in trunk with r238168, test case included.  Thanks!

Bill

> On Jul 8, 2016, at 7:29 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
> 
>> 
>> On Jul 8, 2016, at 12:14 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>> 
>> On Thu, Jul 07, 2016 at 03:40:28PM -0500, Bill Schmidt wrote:
>>>> PR71297 reports that we ICE when __builtin_vec_ld or __builtin_vec_st is
>>>> provided with an incorrect number of arguments.  This patch fixes it by
>>>> bypassing special handling for these intrinsics when the number of
>>>> arguments is wrong, thus allowing the standard error handling for
>>>> builtins to kick in.
>>>> 
>>>> The patch is pretty obvious and I think adding a test case would be
>>>> extraneous, though I can do so if desired.
>> 
>> Well you could use the one from the PR?
>> 
>>>> Bootstrapped and tested on
>>>> powerpc64le-unknown-linux-gnu with no regressions, and the original
>>>> failure is fixed.  Is this ok for trunk?
>> 
>> Yes, but please do a testcase.  Okay for backports, too.
> 
> No backports required; this is a 7 regression.
> 
> Bill
> 
>> 
>> 
>> Segher
>> 
>> 
>>>> 	PR target/71297
>>>> 	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
>>>> 	Allow standard error handling to take over when a wrong number
>>>> 	of arguments is presented to __builtin_vec_ld () or
>>>> 	__builtin_vec_st ().
diff mbox

Patch

Index: gcc/config/rs6000/rs6000-c.c
===================================================================
--- gcc/config/rs6000/rs6000-c.c	(revision 238120)
+++ gcc/config/rs6000/rs6000-c.c	(working copy)
@@ -5281,10 +5281,11 @@  assignment for unaligned loads and stores");
      are able to honor __restrict__, for example.  We may want to
      consider this for all memory access built-ins.
 
-     When -maltivec=be is specified, simply punt to existing
-     built-in processing.  */
+     When -maltivec=be is specified, or the wrong number of arguments
+     is provided, simply punt to existing built-in processing.  */
   if (fcode == ALTIVEC_BUILTIN_VEC_LD
-      && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG))
+      && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG)
+      && nargs == 2)
     {
       tree arg0 = (*arglist)[0];
       tree arg1 = (*arglist)[1];
@@ -5354,7 +5355,8 @@  assignment for unaligned loads and stores");
 
   /* Similarly for stvx.  */
   if (fcode == ALTIVEC_BUILTIN_VEC_ST
-      && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG))
+      && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG)
+      && nargs == 3)
     {
       tree arg0 = (*arglist)[0];
       tree arg1 = (*arglist)[1];