| Message ID | 20121106221813.GZ1881@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 6, 2012 at 11:18 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, Nov 06, 2012 at 02:11:50PM -0800, H.J. Lu wrote: >> On Tue, Nov 6, 2012 at 2:30 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote: >> > Hello, >> >> OK for mainline SVN, please commit. >> > Checked into GCC trunk: http://gcc.gnu.org/ml/gcc-cvs/2012-11/msg00176.html >> > >> > Thanks, K >> >> This caused: >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55224 > > Not only that, it also broke --enable-checking=yes,rtl bootstrap. > SET_DEST isn't valid on CALL, but XEXP (call, 0) is a MEM anyway and > the code looks for reg, so I think looking for CALL was just a mistake. > > This fixes the bootstrap, ok for trunk? > > 2012-11-06 Jakub Jelinek <jakub@redhat.com> > > * config/i386/i386.c (ix86_avx_u128_mode_after): Don't > look for reg in CALL operand. OK. Thanks, Uros.
On Wed, Nov 7, 2012 at 8:03 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> >> OK for mainline SVN, please commit. >>> > Checked into GCC trunk: http://gcc.gnu.org/ml/gcc-cvs/2012-11/msg00176.html >>> > >>> > Thanks, K >>> >>> This caused: >>> >>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55224 >> >> Not only that, it also broke --enable-checking=yes,rtl bootstrap. >> SET_DEST isn't valid on CALL, but XEXP (call, 0) is a MEM anyway and >> the code looks for reg, so I think looking for CALL was just a mistake. >> >> This fixes the bootstrap, ok for trunk? >> >> 2012-11-06 Jakub Jelinek <jakub@redhat.com> >> >> * config/i386/i386.c (ix86_avx_u128_mode_after): Don't >> look for reg in CALL operand. > > OK. You can also break the loop after reg is found. Uros.
On Wed, Nov 07, 2012 at 08:08:08AM +0100, Uros Bizjak wrote: > >> 2012-11-06 Jakub Jelinek <jakub@redhat.com> > >> > >> * config/i386/i386.c (ix86_avx_u128_mode_after): Don't > >> look for reg in CALL operand. > > > > OK. > > You can also break the loop after reg is found. I have committed the patch as is to fix the bootstrap, as anything else needs another bootstrap/regtest. I don't think breaking out of the loop would be correct, then say for *{,sib}call_value_pop patterns reg would be stack pointer rather than the return value of the call. Due to that pattern we can't use single_set, but I wonder if we just can't use XVECEXP (pat, 0, 0) unconditionally for the return value, or perhaps check the condition inside of the loop (REG_P (reg) && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (reg))), return AVX_U128_DIRTY if true (and that way break out of the loop), and return AVX_U128_CLEAN after the loop. Or I wonder why is call handled specially at all, doesn't /* Check if a 256bit AVX register is referenced in stores. */ state = unused; note_stores (pat, check_avx256_stores, &state); if (state == used) return AVX_U128_DIRTY; handle it? Then it would just need to be if (CALL_P (insn)) return AVX_U128_CLEAN. BTW, the formatting is wrong in some spots, e.g. check_avx256_stores (rtx dest, const_rtx set, void *data) { if (((REG_P (dest) || MEM_P(dest)) I'd prefer to leave this to the original submitter. Jakub
Hello, Thanyou for investigation and fixing the problem. I'll answer on remarks later. Regards, Vladimir 2012/11/7 Jakub Jelinek <jakub@redhat.com>: > On Tue, Nov 06, 2012 at 02:11:50PM -0800, H.J. Lu wrote: >> On Tue, Nov 6, 2012 at 2:30 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote: >> > Hello, >> >> OK for mainline SVN, please commit. >> > Checked into GCC trunk: http://gcc.gnu.org/ml/gcc-cvs/2012-11/msg00176.html >> > >> > Thanks, K >> >> This caused: >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55224 > > Not only that, it also broke --enable-checking=yes,rtl bootstrap. > SET_DEST isn't valid on CALL, but XEXP (call, 0) is a MEM anyway and > the code looks for reg, so I think looking for CALL was just a mistake. > > This fixes the bootstrap, ok for trunk? > > 2012-11-06 Jakub Jelinek <jakub@redhat.com> > > * config/i386/i386.c (ix86_avx_u128_mode_after): Don't > look for reg in CALL operand. > > --- gcc/config/i386/i386.c.jj 2012-11-06 18:10:22.000000000 +0100 > +++ gcc/config/i386/i386.c 2012-11-06 20:15:09.068912242 +0100 > @@ -15084,9 +15084,9 @@ ix86_avx_u128_mode_after (int mode, rtx > /* Check for CALL instruction. */ > if (CALL_P (insn)) > { > - if (GET_CODE (pat) == SET || GET_CODE (pat) == CALL) > + if (GET_CODE (pat) == SET) > reg = SET_DEST (pat); > - else if (GET_CODE (pat) == PARALLEL) > + else if (GET_CODE (pat) == PARALLEL) > for (i = XVECLEN (pat, 0) - 1; i >= 0; i--) > { > rtx x = XVECEXP (pat, 0, i); > > > Jakub
I tested changes with configure ../gcc/configure --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --with-fpmath=sse --enable-languages=c,c++,fortran,java,lto,objc --with-arch=corei7-avx --with-cpu=corei7-avx Bootstrap is passed and no new fails in make check. Thank you, Vladimir 2012/11/7 Vladimir Yakovlev <vbyakovl23@gmail.com>: > Hello, > > Thanyou for investigation and fixing the problem. I'll answer on remarks later. > > Regards, > Vladimir > > 2012/11/7 Jakub Jelinek <jakub@redhat.com>: >> On Tue, Nov 06, 2012 at 02:11:50PM -0800, H.J. Lu wrote: >>> On Tue, Nov 6, 2012 at 2:30 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote: >>> > Hello, >>> >> OK for mainline SVN, please commit. >>> > Checked into GCC trunk: http://gcc.gnu.org/ml/gcc-cvs/2012-11/msg00176.html >>> > >>> > Thanks, K >>> >>> This caused: >>> >>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55224 >> >> Not only that, it also broke --enable-checking=yes,rtl bootstrap. >> SET_DEST isn't valid on CALL, but XEXP (call, 0) is a MEM anyway and >> the code looks for reg, so I think looking for CALL was just a mistake. >> >> This fixes the bootstrap, ok for trunk? >> >> 2012-11-06 Jakub Jelinek <jakub@redhat.com> >> >> * config/i386/i386.c (ix86_avx_u128_mode_after): Don't >> look for reg in CALL operand. >> >> --- gcc/config/i386/i386.c.jj 2012-11-06 18:10:22.000000000 +0100 >> +++ gcc/config/i386/i386.c 2012-11-06 20:15:09.068912242 +0100 >> @@ -15084,9 +15084,9 @@ ix86_avx_u128_mode_after (int mode, rtx >> /* Check for CALL instruction. */ >> if (CALL_P (insn)) >> { >> - if (GET_CODE (pat) == SET || GET_CODE (pat) == CALL) >> + if (GET_CODE (pat) == SET) >> reg = SET_DEST (pat); >> - else if (GET_CODE (pat) == PARALLEL) >> + else if (GET_CODE (pat) == PARALLEL) >> for (i = XVECLEN (pat, 0) - 1; i >= 0; i--) >> { >> rtx x = XVECEXP (pat, 0, i); >> >> >> Jakub
--- gcc/config/i386/i386.c.jj 2012-11-06 18:10:22.000000000 +0100 +++ gcc/config/i386/i386.c 2012-11-06 20:15:09.068912242 +0100 @@ -15084,9 +15084,9 @@ ix86_avx_u128_mode_after (int mode, rtx /* Check for CALL instruction. */ if (CALL_P (insn)) { - if (GET_CODE (pat) == SET || GET_CODE (pat) == CALL) + if (GET_CODE (pat) == SET) reg = SET_DEST (pat); - else if (GET_CODE (pat) == PARALLEL) + else if (GET_CODE (pat) == PARALLEL) for (i = XVECLEN (pat, 0) - 1; i >= 0; i--) { rtx x = XVECEXP (pat, 0, i);