Message ID | 1467918661.18357.5.camel@oc8801110288.ibm.com |
---|---|
State | New |
Headers | show |
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]; >
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 ().
> 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 (). >
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 ().
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];