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

[Uros Bizjak]

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.
-------------- next part --------------
Index: testsuite/gcc.target/i386/pr60851.c
===================================================================
--- testsuite/gcc.target/i386/pr60851.c	(revision 0)
+++ testsuite/gcc.target/i386/pr60851.c	(revision 0)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -flive-range-shrinkage -mtune=bdver4 -mdispatch-scheduler" } */
+
+long double ld (char c)
+{
+  return c;
+}
Index: recog.c
===================================================================
--- recog.c	(revision 221587)
+++ recog.c	(working copy)
@@ -2627,9 +2627,14 @@ constrain_operands (int strict)
 		      break;
 		    win = 1;
 		  }
-		/* Before reload, accept what reload can turn into mem.  */
+		/* Before reload, accept what reload can turn into a mem.  */
 		else if (strict < 0 && CONSTANT_P (op))
 		  win = 1;
+		/* Before reload, accept a pseudo,
+		   since LRA can turn it into a mem.  */
+		else if (strict < 0 && targetm.lra_p () && REG_P (op)
+			 && REGNO (op) >= FIRST_PSEUDO_REGISTER)
+		  win = 1;
 		/* During reload, accept a pseudo  */
 		else if (reload_in_progress && REG_P (op)
 			 && REGNO (op) >= FIRST_PSEUDO_REGISTER)
@@ -2708,6 +2713,10 @@ constrain_operands (int strict)
 		    /* Before reload, accept what reload can handle.  */
 		    || (strict < 0
 			&& (CONSTANT_P (op) || MEM_P (op)))
+		    /* 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)
 		    /* During reload, accept a pseudo  */
 		    || (reload_in_progress && REG_P (op)
 			&& REGNO (op) >= FIRST_PSEUDO_REGISTER))
@@ -2739,8 +2748,12 @@ constrain_operands (int strict)
 			   /* Every memory operand can be reloaded to fit.  */
 			   && ((strict < 0 && MEM_P (op))
 			       /* Before reload, accept what reload can turn
-				  into mem.  */
+				  into a mem.  */
 			       || (strict < 0 && CONSTANT_P (op))
+			       /* 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)
 			       /* During reload, accept a pseudo  */
 			       || (reload_in_progress && REG_P (op)
 				   && REGNO (op) >= FIRST_PSEUDO_REGISTER)))