Re: PATCH: ICE: SIGSEGV in decide_alg() with -mmemset-strategy=libcall:-1:align -minline-all-stringops

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Dec 2, 2014 at 6:05 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> 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.

This is what I checked in.  I will wait for a few days before backporting
it to 4.9.

Thanks.

-- 
H.J.
---
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 3eacc5a..c746d0f 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2014-12-02  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.
+
 2014-12-02  Tom de Vries  <tom@codesourcery.com>

  PR rtl-optimization/63718
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3397167..211c9e6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -24464,7 +24464,8 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT
expected_size,
       *noalign = alg_noalign;
       return alg;
     }
-  break;
+  else if (!any_alg_usable_p)
+    break;
  }
       else if (alg_usable_p (candidate, memset))
  {
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 00da0bd..5d66849 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-12-02  H.J. Lu  <hongjiu.lu@intel.com>
+
+ PR target/64108
+ * gcc.target/i386/memset-strategy-2.c: New test.
+
 2014-12-02  Richard Biener  <rguenther@suse.de>

  * gcc.dg/torture/20141202-1.c: New testcase.
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);
+}
+