This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: Regression in target MIC compiler
- From: "David Sherwood" <david dot sherwood at arm dot com>
- To: "'Thomas Schwinge'" <thomas at codesourcery dot com>, "Ilya Verbin" <iverbin at gmail dot com>, "Jeff Law" <law at redhat dot com>
- Cc: "Jakub Jelinek" <jakub at redhat dot com>, <gcc-patches at gcc dot gnu dot org>, "Kirill Yukhin" <kirill dot yukhin at gmail dot com>, <nathan at codesourcery dot com>, "Richard Sandiford" <rdsandiford at googlemail dot com>
- Date: Wed, 5 Aug 2015 11:18:32 +0100
- Subject: RE: Regression in target MIC compiler
- Authentication-results: sourceware.org; auth=none
- References: <20150731162758 dot GB64740 at msticlxl57 dot ims dot intel dot com> <20150731165316 dot GC64740 at msticlxl57 dot ims dot intel dot com> <20150731165959 dot GI1780 at tucnak dot redhat dot com> <20150731171302 dot GD64740 at msticlxl57 dot ims dot intel dot com> <87d1z3mdts dot fsf at kepler dot schwinge dot homeip dot net> <000101d0c926$60aa5420$21fefc60$ at arm dot com> <20150804130623 dot GA31722 at msticlxl57 dot ims dot intel dot com> <87egjiw086 dot fsf at schwinge dot name>
Hi Thomas,
If this looks like my fault I am happy to look into this and fix the bug
if you can tell me how to reproduce it. I recently changed GET_MODE_INNER (m)
to return 'm' itself if there is no inner mode and I thought I'd fixed up lto,
but it seems I got it wrong. It also sounds like there is another bug in this
area too - if I want to test this do I need to apply any other patches too?
Regards,
David.
>
> Hi!
>
> It seems as if David's Â[PATCH][1/N] Change GET_MODE_INNER to always
> return a non-void mode is relevant here:
>
> On Tue, 4 Aug 2015 16:06:23 +0300, Ilya Verbin <iverbin@gmail.com> wrote:
> > On Tue, Aug 04, 2015 at 14:35:11 +0200, Thomas Schwinge wrote:
> > > On Fri, 31 Jul 2015 20:13:02 +0300, Ilya Verbin <iverbin@gmail.com> wrote:
> > > > On Fri, Jul 31, 2015 at 18:59:59 +0200, Jakub Jelinek wrote:
> > > > > > > On Wed, Feb 18, 2015 at 11:00:35 +0100, Jakub Jelinek wrote:
> > > > > > > + /* First search just the GET_CLASS_NARROWEST_MODE to wider modes,
> > > > > > > + if not found, fallback to all modes. */
> > > > > > > + int pass;
> > > > > > > + for (pass = 0; pass < 2; pass++)
> > > > > > > + for (machine_mode mr = pass ? VOIDmode
> > > > > > > + : GET_CLASS_NARROWEST_MODE (mclass);
> > > > > > > + pass ? mr < MAX_MACHINE_MODE : mr != VOIDmode;
> > > > > > > + pass ? mr = (machine_mode) (m + 1)
> > > > > > > + : mr = GET_MODE_WIDER_MODE (mr))
> > > > > > > + if (GET_MODE_CLASS (mr) != mclass
> > > > > > > + || GET_MODE_SIZE (mr) != size
> > > > > > > + || GET_MODE_PRECISION (mr) != prec
> > > > > > > + || GET_MODE_INNER (mr) != inner
> > > > > > > + || GET_MODE_IBIT (mr) != ibit
> > > > > > > + || GET_MODE_FBIT (mr) != fbit
> > > > > > > + || GET_MODE_NUNITS (mr) != nunits)
> > > > > > > + continue;
> > > > > > >
> > > > > > > Given that gomp-4_1-branch works ok, the problem was introduced somewhere
> > > > > > > between 9 and 31 Jul. I'll try to find the revision.
> > > > > >
> > > > > > Shouldn't 'mr' be here instead of 'm'?
> > > > >
> > > > > I think so. If it works, patch preapproved.
> > > >
> > > > It fixes the infinite loop, but causes an error:
> > > > lto1: fatal error: unsupported mode QI
> > >
> > > Confirmed.
> > >
> > > > > But wonder what changed that we haven't been triggering it before.
> > > > > What mode do you think it on (mclass/size/prec/inner/ibit/fbit/nunits)?
> > > >
> > > > When in hangs, mr is HImode.
> > >
> > > Do you already have any further analysis, a workaround, or even a fix?
> >
> > Not yet. I thought since Jakub is the author of this function, he could easily
> > point what is wrong here :) Actually, intelmic doesn't require
> > lto_input_mode_table, so temporary workaround is just to disable it.
>
> Well, avoiding lto_input_mode_table doesn't help us with nvptx
> offloading... ;-)
>
> Anyway, I found that if I revert r226328, Â[PATCH][1/N] Change
> GET_MODE_INNER to always return a non-void modeÂ,
> <https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02350.html>, the problem
> goes away. I'm trying, but I cannot claim yet to really understand this
> mode streaming code... But, with the producer
> gcc/lto-streamer-out.c:lto_write_mode_table having been changed, does
> maybe the consumer gcc/lto-streamer-in.c:lto_input_mode_table also need
> to be updated accordingly?
>
> For reference, David's change to
> gcc/lto-streamer-out.c:lto_write_mode_table:
>
> @@ -2679,23 +2679,23 @@ lto_write_mode_table (void)
> /* Ensure that for GET_MODE_INNER (m) != VOIDmode we have
> also the inner mode marked. */
> for (int i = 0; i < (int) MAX_MACHINE_MODE; i++)
> if (streamer_mode_table[i])
> {
> machine_mode m = (machine_mode) i;
> - if (GET_MODE_INNER (m) != VOIDmode)
> + if (GET_MODE_INNER (m) != m)
> streamer_mode_table[(int) GET_MODE_INNER (m)] = 1;
> }
> /* First stream modes that have GET_MODE_INNER (m) == VOIDmode,
> so that we can refer to them afterwards. */
> for (int pass = 0; pass < 2; pass++)
> for (int i = 0; i < (int) MAX_MACHINE_MODE; i++)
> if (streamer_mode_table[i] && i != (int) VOIDmode && i != (int) BLKmode)
> {
> machine_mode m = (machine_mode) i;
> - if ((GET_MODE_INNER (m) == VOIDmode) ^ (pass == 0))
> + if ((GET_MODE_INNER (m) == m) ^ (pass == 0))
> continue;
> bp_pack_value (&bp, m, 8);
> bp_pack_enum (&bp, mode_class, MAX_MODE_CLASS, GET_MODE_CLASS (m));
> bp_pack_value (&bp, GET_MODE_SIZE (m), 8);
> bp_pack_value (&bp, GET_MODE_PRECISION (m), 16);
> bp_pack_value (&bp, GET_MODE_INNER (m), 8);
>
> (Also, the source code comments need to be updated?)
>
>
> GrÃÃe,
> Thomas