| Message ID | 20141129140030.GA10082@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Nov 29, 2014 at 6:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > When searching for an usable algorithm with -minline-all-stringops, > decide_alg stops when it sees libcall even if there is a usable > algorithm. It goes into an infinite loop. This patch changes > decide_alg to stop searching only if there aren't any usable algorithms. > Testd on Linux/x86-64. OK for trunk. The bug is also in GCC 4.9. OK for backport? > > H.J. > ---- > gcc/ > > 2014-11-29 H.J. Lu <hongjiu.lu@intel.com> > > PR target/64108 > * config/i386/i386.c (decide_alg): Stop only if there aren't > any usable algorithms. > > gcc/testsuite/ > > 2014-11-29 H.J. Lu <hongjiu.lu@intel.com> > > PR target/64108 > * gcc.target/i386/memset-strategy-2.c: New test. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 2493130..d789635 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -24464,7 +24464,9 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size, > *noalign = alg_noalign; > return alg; > } > - break; > + /* Stop only if there aren't any usable algorithms. */ > + if (!any_alg_usable_p) > + break; > } > else if (alg_usable_p (candidate, memset)) > { > diff --git a/gcc/testsuite/gcc.target/i386/memset-strategy-2.c b/gcc/testsuite/gcc.target/i386/memset-strategy-2.c > new file mode 100644 > index 0000000..aafa54d > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/memset-strategy-2.c > @@ -0,0 +1,10 @@ > +/* PR target/64108 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -march=atom -mmemset-strategy=libcall:-1:align -minline-all-stringops" } */ > + > +char a[2048]; > +void t (void) > +{ > + __builtin_memset (a, 1, 2048); > +} > +
On Sat, Nov 29, 2014 at 3:00 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > When searching for an usable algorithm with -minline-all-stringops, > decide_alg stops when it sees libcall even if there is a usable > algorithm. It goes into an infinite loop. This patch changes > decide_alg to stop searching only if there aren't any usable algorithms. > Testd on Linux/x86-64. OK for trunk. > > > H.J. > ---- > gcc/ > > 2014-11-29 H.J. Lu <hongjiu.lu@intel.com> > > PR target/64108 > * config/i386/i386.c (decide_alg): Stop only if there aren't > any usable algorithms. > > gcc/testsuite/ > > 2014-11-29 H.J. Lu <hongjiu.lu@intel.com> > > PR target/64108 > * gcc.target/i386/memset-strategy-2.c: New test. OK with a change below. Please wait a couple of days before backporting the fix - the fix looks kind of obvious, but this part of the code is somehow complex and has biten us in the past. Thanks, Uros. > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 2493130..d789635 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -24464,7 +24464,9 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size, > *noalign = alg_noalign; > return alg; > } > - break; > + /* Stop only if there aren't any usable algorithms. */ > + if (!any_alg_usable_p) > + break; Please use "else if ..." and remove unneded argument. IMO, the code speaks for itself. > } > else if (alg_usable_p (candidate, memset)) > { > diff --git a/gcc/testsuite/gcc.target/i386/memset-strategy-2.c b/gcc/testsuite/gcc.target/i386/memset-strategy-2.c > new file mode 100644 > index 0000000..aafa54d > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/memset-strategy-2.c > @@ -0,0 +1,10 @@ > +/* PR target/64108 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -march=atom -mmemset-strategy=libcall:-1:align -minline-all-stringops" } */ > + > +char a[2048]; > +void t (void) > +{ > + __builtin_memset (a, 1, 2048); > +} > +
On Tue, Dec 2, 2014 at 5:25 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Sat, Nov 29, 2014 at 3:00 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> When searching for an usable algorithm with -minline-all-stringops, >> decide_alg stops when it sees libcall even if there is a usable >> algorithm. It goes into an infinite loop. This patch changes >> decide_alg to stop searching only if there aren't any usable algorithms. >> Testd on Linux/x86-64. OK for trunk. >> >> >> H.J. >> ---- >> gcc/ >> >> 2014-11-29 H.J. Lu <hongjiu.lu@intel.com> >> >> PR target/64108 >> * config/i386/i386.c (decide_alg): Stop only if there aren't >> any usable algorithms. >> >> gcc/testsuite/ >> >> 2014-11-29 H.J. Lu <hongjiu.lu@intel.com> >> >> PR target/64108 >> * gcc.target/i386/memset-strategy-2.c: New test. > > OK with a change below. Please wait a couple of days before > backporting the fix - the fix looks kind of obvious, but this part of > the code is somehow complex and has biten us in the past. Will do. > Thanks, > Uros. > >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index 2493130..d789635 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -24464,7 +24464,9 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size, >> *noalign = alg_noalign; >> return alg; >> } >> - break; >> + /* Stop only if there aren't any usable algorithms. */ >> + if (!any_alg_usable_p) >> + break; > > Please use "else if ..." and remove unneded argument. IMO, the code > speaks for itself. Which unneeded argument to remove? All arguments of decide_alg are used. >> } >> else if (alg_usable_p (candidate, memset)) >> { >> diff --git a/gcc/testsuite/gcc.target/i386/memset-strategy-2.c b/gcc/testsuite/gcc.target/i386/memset-strategy-2.c >> new file mode 100644 >> index 0000000..aafa54d >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/i386/memset-strategy-2.c >> @@ -0,0 +1,10 @@ >> +/* PR target/64108 */ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -march=atom -mmemset-strategy=libcall:-1:align -minline-all-stringops" } */ >> + >> +char a[2048]; >> +void t (void) >> +{ >> + __builtin_memset (a, 1, 2048); >> +} >> +
On Tue, Dec 2, 2014 at 3:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Dec 2, 2014 at 5:25 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Sat, Nov 29, 2014 at 3:00 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> When searching for an usable algorithm with -minline-all-stringops, >>> decide_alg stops when it sees libcall even if there is a usable >>> algorithm. It goes into an infinite loop. This patch changes >>> decide_alg to stop searching only if there aren't any usable algorithms. >>> Testd on Linux/x86-64. OK for trunk. >>> >>> >>> H.J. >>> ---- >>> gcc/ >>> >>> 2014-11-29 H.J. Lu <hongjiu.lu@intel.com> >>> >>> PR target/64108 >>> * config/i386/i386.c (decide_alg): Stop only if there aren't >>> any usable algorithms. >>> >>> gcc/testsuite/ >>> >>> 2014-11-29 H.J. Lu <hongjiu.lu@intel.com> >>> >>> PR target/64108 >>> * gcc.target/i386/memset-strategy-2.c: New test. >> >> OK with a change below. Please wait a couple of days before >> backporting the fix - the fix looks kind of obvious, but this part of >> the code is somehow complex and has biten us in the past. > > Will do. > >> Thanks, >> Uros. >> >>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >>> index 2493130..d789635 100644 >>> --- a/gcc/config/i386/i386.c >>> +++ b/gcc/config/i386/i386.c >>> @@ -24464,7 +24464,9 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size, >>> *noalign = alg_noalign; >>> return alg; >>> } >>> - break; >>> + /* Stop only if there aren't any usable algorithms. */ >>> + if (!any_alg_usable_p) >>> + break; >> >> Please use "else if ..." and remove unneded argument. IMO, the code >> speaks for itself. > > Which unneeded argument to remove? All arguments of decide_alg are > used. Eh, slip of the tongue. Unnedded *comment*, the one your patch is adding. Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 2493130..d789635 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -24464,7 +24464,9 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size, *noalign = alg_noalign; return alg; } - break; + /* Stop only if there aren't any usable algorithms. */ + if (!any_alg_usable_p) + break; } else if (alg_usable_p (candidate, memset)) { diff --git a/gcc/testsuite/gcc.target/i386/memset-strategy-2.c b/gcc/testsuite/gcc.target/i386/memset-strategy-2.c new file mode 100644 index 0000000..aafa54d --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/memset-strategy-2.c @@ -0,0 +1,10 @@ +/* PR target/64108 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=atom -mmemset-strategy=libcall:-1:align -minline-all-stringops" } */ + +char a[2048]; +void t (void) +{ + __builtin_memset (a, 1, 2048); +} +