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

Hello!

Attached patch activates "safety net" recognition of pseudo regs for
LRA-enabled targets. LRA is able to spill pseudos to memory to satisfy
memory-only constraints, so constrain_operands should recognize this
functionality.

This is actually the same approach as how constants are handled a
couple of lines above. the changed section.

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.

testsuite/ChangeLog:

2015-03-18  Uros Bizjak  <ubizjak@gmail.com>

    PR rtl-optimization/60851
    * gcc.target/i386/pr60851.c: New test.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

OK for mainline and 4.9 branch?

Uros.

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.
> 
> --- recog.c	(revision 221493)
> +++ recog.c	(working copy)
> @@ -2775,6 +2775,10 @@ constrain_operands (int strict, alternative_mask a
>  			       /* Before reload, accept what reload can turn
>  				  into mem.  */
>  			       || (strict < 0 && CONSTANT_P (op))
> +			       /* Before reload, accept a pseudo,
> +				  since LRA can turn it into mem.  */
> +			       || (targetm.lra_p () && strict < 0 && REG_P (op)
> +				   && REGNO (op) >= FIRST_PSEUDO_REGISTER)
>  			       /* During reload, accept a pseudo  */
>  			       || (reload_in_progress && REG_P (op)
>  				   && REGNO (op) >= FIRST_PSEUDO_REGISTER)))

This looks reasonable to me, but please give Vlad an extra day to comment on
it.

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.

For 4.9 backport, please wait a few days after it goes into the trunk.

	Jakub

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.

Thanks!

Uros.