| Message ID | ee053cb8-1618-a869-6cd3-d8be2953d331@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jul 31, 2017 at 11:19:26AM -0500, Bill Schmidt wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81622 reports an ICE-on-invalid > for the vec_ld and vec_st built-in functions. This fires when the last > argument of the built-in is not a pointer or array type, as is required. > We break on this during early expansion of the built-ins into tree code > during parsing. The solution, as with other ill-formed uses of these > built-ins, is to avoid the early expansion when the argument has an invalid > type, so that normal error handling can kick in later. > > (The long-term solution is to move the vec_ld and vec_st built-ins to the > gimple folding work that Will Schmidt has been doing, but that hasn't > happened yet.) > > Bootstrapped and tested on powerpc64le-linux-gnu with no regressions. > Is this ok for trunk and GCC 7? I'd like to get it into 7.2 since it > is a 7 regression. See the patch I've attached in the PR, this isn't sufficient (and for the ARRAY_TYPE I wonder if you can ever see there ARRAY_TYPE), the function has various other issues, including e.g. ICE on vec_cmpne (1, 2) with -mpower9. Jakub
> On Jul 31, 2017, at 11:27 AM, Jakub Jelinek <jakub@redhat.com> wrote: > > On Mon, Jul 31, 2017 at 11:19:26AM -0500, Bill Schmidt wrote: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81622 reports an ICE-on-invalid >> for the vec_ld and vec_st built-in functions. This fires when the last >> argument of the built-in is not a pointer or array type, as is required. >> We break on this during early expansion of the built-ins into tree code >> during parsing. The solution, as with other ill-formed uses of these >> built-ins, is to avoid the early expansion when the argument has an invalid >> type, so that normal error handling can kick in later. >> >> (The long-term solution is to move the vec_ld and vec_st built-ins to the >> gimple folding work that Will Schmidt has been doing, but that hasn't >> happened yet.) >> >> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions. >> Is this ok for trunk and GCC 7? I'd like to get it into 7.2 since it >> is a 7 regression. > > See the patch I've attached in the PR, this isn't sufficient > (and for the ARRAY_TYPE I wonder if you can ever see there ARRAY_TYPE), > the function has various other issues, including e.g. ICE on > vec_cmpne (1, 2) with -mpower9. Yes, I'll withdraw the patch (but the ARRAY_TYPE thing is necessary as we've discussed). I'll step out of your way on this one since you've got it well in hand. It would be great to have a fix in for 7.2, though. Bill > > Jakub >
Index: gcc/config/rs6000/rs6000-c.c =================================================================== --- gcc/config/rs6000/rs6000-c.c (revision 250721) +++ gcc/config/rs6000/rs6000-c.c (working copy) @@ -6454,10 +6454,13 @@ altivec_resolve_overloaded_builtin (location_t loc consider this for all memory access built-ins. When -maltivec=be is specified, or the wrong number of arguments - is provided, simply punt to existing built-in processing. */ + is provided, or the second argument isn't a pointer, simply punt + to existing built-in processing. */ if (fcode == ALTIVEC_BUILTIN_VEC_LD && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG) - && nargs == 2) + && nargs == 2 + && (POINTER_TYPE_P ((*arglist)[1]) + || TREE_CODE (TREE_TYPE ((*arglist)[1])) == ARRAY_TYPE)) { tree arg0 = (*arglist)[0]; tree arg1 = (*arglist)[1]; @@ -6528,7 +6531,9 @@ altivec_resolve_overloaded_builtin (location_t loc /* Similarly for stvx. */ if (fcode == ALTIVEC_BUILTIN_VEC_ST && (BYTES_BIG_ENDIAN || !VECTOR_ELT_ORDER_BIG) - && nargs == 3) + && nargs == 3 + && (POINTER_TYPE_P ((*arglist)[2]) + || TREE_CODE (TREE_TYPE ((*arglist)[2])) == ARRAY_TYPE)) { tree arg0 = (*arglist)[0]; tree arg1 = (*arglist)[1]; Index: gcc/testsuite/gcc.target/powerpc/pr81622.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pr81622.c (nonexistent) +++ gcc/testsuite/gcc.target/powerpc/pr81622.c (working copy) @@ -0,0 +1,11 @@ +/* { dg-do compile } */ + +void a() +{ + __builtin_vec_ld (1, 2); /* { dg-error "invalid parameter combination for AltiVec intrinsic __builtin_vec_ld" } */ +} + +void b() +{ + __builtin_vec_st (0, 1, 2); /* { dg-error "invalid parameter combination for AltiVec intrinsic __builtin_vec_st" } */ +}