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

[Uros Bizjak <ubizjak at gmail dot com>]

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.