Re: [PATCH, rtl-optimization]: Fix PR60851, ICE: in extract_constrain_insn_cached, at recog.c

[Uros Bizjak <ubizjak at gmail dot com>]

On Thu, Mar 19, 2015 at 9:38 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Mar 19, 2015 at 8:44 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Wed, Mar 18, 2015 at 08:00:33PM +0100, Uros Bizjak wrote:
>>> 2015-03-18  Uros Bizjak  <ubizjak@gmail.com>
>>>
>>>     PR rtl-optimization/60851
>>>     * recog.c (constrain_operands): Accept a pseudo register before reload
>>>     for LRA enabled targets.
>
>> As the two adjacent conditions are mostly the same, perhaps it might be
>> better written as
>>                                || ((/* Before reload, accept a pseudo, since
>>                                        LRA can turn it into a mem.
>>                                     (targetm.lra_p () && strict < 0)
>>                                     /* During reload, accept a pseudo.  */
>>                                     || reload_in_progress)
>>                                    && REG_P (op)
>>                                    && REGNO (op) >= FIRST_PSEUDO_REGISTER)))
>>
>> or put REG_P && REGNO checks first and only then test when.
>
> Yeah, I thought about this particular CSE a bit. But since these are
> two conceptually different conditions, and the formatting (and
> comments) just didn't fit into available space, I wrote it this way.
> IMO, it is more readable, better follows the intended logic, and
> avoids even more indents.
>
> Also, I am pretty sure that any decent compiler can CSE this part on its own.
>
> However, the condition can be slightly improved by rewriting it to:
>
>                    /* Before reload, accept a pseudo,
>                   since LRA can turn it into a mem.  */
>                    || (strict < 0 && targetm.lra_p () && REG_P (op)
>                    && REGNO (op) >= FIRST_PSEUDO_REGISTER)
>
> so, we have cheaper tests in the front to shortcut more expensive tests later.
>
>> For 4.9 backport, please wait a few days after it goes into the trunk.

Attached is the 4.9 branch version of the patch. There were
substantial cleanups [1] in this area for 5.0, so comparing to trunk
version, the patch includes two more places that have to be changed.

Patch was bootstrapped and regression tested on x86_64-linux-gnu
{,-m32}. If there are no objections, I plan to commit this version to
the branch in a couple of days.

[1] https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00530.html

Uros.

Attachment: p-49.diff.txt
Description: Text document