Re: [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

Re: [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Bernd Edlinger]

FYI: I have a patch for the aarch64 regression here:
https://gcc.gnu.org/bugzilla/attachment.cgi?id=39600

It passes bootstrap and reg-testing on x86_64-linux-gnu.

Also the mentioned aarch64 and powerpc test cases
pass manually in a cross-compiler, but I cannot do the
boot-strap on powerpc or aarch64 by myself.

So It would be nice if one of you would take the time
and try that patch.


Thanks
Bernd.

Re: [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Peter Bergner]

On 9/11/16 3:35 PM, Bernd Edlinger wrote:
> FYI: I have a patch for the aarch64 regression here:
> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39600
>
> It passes bootstrap and reg-testing on x86_64-linux-gnu.

Thanks for debugging and fixing this!


> Also the mentioned aarch64 and powerpc test cases
> pass manually in a cross-compiler, but I cannot do the
> boot-strap on powerpc or aarch64 by myself.
>
> So It would be nice if one of you would take the time
> and try that patch.

I can confirm that is bootstraps and regtests with no regressions
on powerpc64le-linux and that the pr77289.c test case does pass.

Peter

Re: [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Kyrill Tkachov]

On 11/09/16 21:53, Peter Bergner wrote:
> On 9/11/16 3:35 PM, Bernd Edlinger wrote:
>> FYI: I have a patch for the aarch64 regression here:
>> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39600
>>
>> It passes bootstrap and reg-testing on x86_64-linux-gnu.
>
> Thanks for debugging and fixing this!
>
>
>> Also the mentioned aarch64 and powerpc test cases
>> pass manually in a cross-compiler, but I cannot do the
>> boot-strap on powerpc or aarch64 by myself.
>>
>> So It would be nice if one of you would take the time
>> and try that patch.
>
> I can confirm that is bootstraps and regtests with no regressions
> on powerpc64le-linux and that the pr77289.c test case does pass.
>

The patch also passes bootstrap and testing on aarch64-none-linux-gnu.
The ICEs I was seeing go away.

Thanks!
Kyrill

> Peter
>
>
>

[PATCH] Fix aarch64 fallout of [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 158 bytes --]

Hi,

the attached patch boot-straps and reg-tests cleanly
on x86_64-pc-linux-gnu, powerpc and aarch64 targets.

Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-pr77289.diff --]
[-- Type: text/x-patch; name="patch-pr77289.diff", Size: 4027 bytes --]

2016-09-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR rtl-optimization/77289
	* lra-constraints.c (get_final_hard_regno): Removed.
	(get_hard_regno): Add new parameter final_p.
	(get_reg_class): Directly call lra_get_elimination_hard_regno.
	(operands_match_p): Adjust call to get_hard_regno.
	(uses_hard_regs_p): Likewise.
	(process_alt_operands): Likewise.


Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	(revision 240073)
+++ gcc/lra-constraints.c	(working copy)
@@ -182,27 +182,15 @@ get_try_hard_regno (int regno)
   return ira_class_hard_regs[rclass][0];
 }
 
-/* Return the final hard regno which will be after elimination.
-   We do this because the final hard regno could have a different class.  */
-static int
-get_final_hard_regno (int regno)
-{
-  if (! HARD_REGISTER_NUM_P (regno))
-    regno = lra_get_regno_hard_regno (regno);
-  if (regno < 0)
-    return regno;
-  return lra_get_elimination_hard_regno (regno);
-}
-
 /* Return the hard regno of X after removing its subreg.  If X is not
    a register or a subreg of a register, return -1.  If X is a pseudo,
-   use its assignment.  We do not process register eliminiations while
-   matching constraints.  See PR77289.  */
+   use its assignment.  If FINAL_P return the final hard regno which will
+   be after elimination.  */
 static int
-get_hard_regno (rtx x)
+get_hard_regno (rtx x, bool final_p)
 {
   rtx reg;
-  int offset, hard_regno;
+  int hard_regno;
 
   reg = x;
   if (SUBREG_P (x))
@@ -213,11 +201,12 @@ static int
     hard_regno = lra_get_regno_hard_regno (hard_regno);
   if (hard_regno < 0)
     return -1;
-  offset = 0;
+  if (final_p)
+    hard_regno = lra_get_elimination_hard_regno (hard_regno);
   if (SUBREG_P (x))
-    offset += subreg_regno_offset (hard_regno, GET_MODE (reg),
-				   SUBREG_BYTE (x),  GET_MODE (x));
-  return hard_regno + offset;
+    hard_regno += subreg_regno_offset (hard_regno, GET_MODE (reg),
+				       SUBREG_BYTE (x),  GET_MODE (x));
+  return hard_regno;
 }
 
 /* If REGNO is a hard register or has been allocated a hard register,
@@ -229,11 +218,11 @@ get_reg_class (int regno)
 {
   int hard_regno;
 
-  if ((hard_regno = regno) >= FIRST_PSEUDO_REGISTER)
+  if (! HARD_REGISTER_NUM_P (hard_regno = regno))
     hard_regno = lra_get_regno_hard_regno (regno);
   if (hard_regno >= 0)
     {
-      hard_regno = get_final_hard_regno (hard_regno);
+      hard_regno = lra_get_elimination_hard_regno (hard_regno);
       return REGNO_REG_CLASS (hard_regno);
     }
   if (regno >= new_regno_start)
@@ -694,7 +683,7 @@ operands_match_p (rtx x, rtx y, int y_hard_regno)
     {
       int j;
 
-      i = get_hard_regno (x);
+      i = get_hard_regno (x, false);
       if (i < 0)
 	goto slow;
 
@@ -1713,7 +1702,7 @@ uses_hard_regs_p (rtx x, HARD_REG_SET set)
 
   if (REG_P (x))
     {
-      x_hard_regno = get_final_hard_regno (REGNO (x));
+      x_hard_regno = get_hard_regno (x, true);
       return (x_hard_regno >= 0
 	      && overlaps_hard_reg_set_p (set, mode, x_hard_regno));
     }
@@ -1840,7 +1829,7 @@ process_alt_operands (int only_alternative)
 
       op = no_subreg_reg_operand[nop] = *curr_id->operand_loc[nop];
       /* The real hard regno of the operand after the allocation.  */
-      hard_regno[nop] = get_hard_regno (op);
+      hard_regno[nop] = get_hard_regno (op, true);
 
       operand_reg[nop] = reg = op;
       biggest_mode[nop] = GET_MODE (op);
@@ -2001,7 +1990,7 @@ process_alt_operands (int only_alternative)
 		    lra_assert (nop > m);
 
 		    this_alternative_matches = m;
-		    m_hregno = get_hard_regno (*curr_id->operand_loc[m]);
+		    m_hregno = get_hard_regno (*curr_id->operand_loc[m], false);
 		    /* We are supposed to match a previous operand.
 		       If we do, we win if that one did.  If we do
 		       not, count both of the operands as losers.

Re: [PATCH] Fix aarch64 fallout of [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Jeff Law]

On 09/12/2016 07:43 AM, Bernd Edlinger wrote:
> Hi,
>
> the attached patch boot-straps and reg-tests cleanly
> on x86_64-pc-linux-gnu, powerpc and aarch64 targets.
>
> Is it OK for trunk?
Unfortunately, there's no explanation for why the original code was 
wrong.  Yes, I can clearly see you're trying to fix an ICE, but what 
about the code's behaviour was wrong?

jeff

Re: [PATCH] Fix aarch64 fallout of [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Bernd Edlinger]

On 09/12/16 23:45, Jeff Law wrote:
> On 09/12/2016 07:43 AM, Bernd Edlinger wrote:
>> Hi,
>>
>> the attached patch boot-straps and reg-tests cleanly
>> on x86_64-pc-linux-gnu, powerpc and aarch64 targets.
>>
>> Is it OK for trunk?
> Unfortunately, there's no explanation for why the original code was
> wrong.  Yes, I can clearly see you're trying to fix an ICE, but what
> about the code's behaviour was wrong?
>
> jeff
>

Sorry,

everything is just folding, and continuing Peter's cleanup,
except here:

@@ -1840,7 +1829,7 @@ process_alt_operands (int only_alternative)

        op = no_subreg_reg_operand[nop] = *curr_id->operand_loc[nop];
        /* The real hard regno of the operand after the allocation.  */
-      hard_regno[nop] = get_hard_regno (op);
+      hard_regno[nop] = get_hard_regno (op, true);

        operand_reg[nop] = reg = op;
        biggest_mode[nop] = GET_MODE (op);


note the comment above.  This restores the state before Peter's patch
here, but only here, not in the operand matching.



Bernd.

Re: [PATCH] Fix aarch64 fallout of [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Jeff Law]

On 09/12/2016 07:43 AM, Bernd Edlinger wrote:
> Hi,
>
> the attached patch boot-straps and reg-tests cleanly
> on x86_64-pc-linux-gnu, powerpc and aarch64 targets.
>
> Is it OK for trunk?
>
>
> Thanks
> Bernd.
>
>
> patch-pr77289.diff
>
>
> 2016-09-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	PR rtl-optimization/77289
> 	* lra-constraints.c (get_final_hard_regno): Removed.
> 	(get_hard_regno): Add new parameter final_p.
> 	(get_reg_class): Directly call lra_get_elimination_hard_regno.
> 	(operands_match_p): Adjust call to get_hard_regno.
> 	(uses_hard_regs_p): Likewise.
> 	(process_alt_operands): Likewise.
A bit painful to follow as there's cleanup mixed in with the bugfix. 
Thanks for pointing out the key change in the follow-up, that helped 
considerably.

OK.

jeff
>

Re: [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Andrew Pinski]

On Fri, Sep 9, 2016 at 6:37 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> On 9/9/16 5:51 PM, Jeff Law wrote:
>>
>> On 08/30/2016 10:23 PM, Peter Bergner wrote:
>>>
>>> gcc/
>>>     PR rtl-optimization/77289
>>>     * lra-constraints.c (get_final_hard_regno): Add support for non hard
>>>     register numbers.  Remove support for subregs.
>>>     (get_hard_regno): Use SUBREG_P.  Don't call get_final_hard_regno().
>>>     (get_reg_class): Delete removed get_final_hard_regno() argument.
>>>     (uses_hard_regs_p): Call get_final_hard_regno().
>>>
>>> gcc/testsuite/
>>>     PR rtl-optimization/77289
>>>     * gcc.target/powerpc/pr77289.c: New test.
>>
>> OK.
>
>
> Committed as revision 240065.  Thanks!

This patch caused many regressions on aarch64-linux-gnu:
gcc.c-torture/compile/20020604-1.c   -O1  (test for excess errors)
gcc.c-torture/compile/complex-6.c   -O2  (test for excess errors)
gcc.c-torture/compile/complex-6.c   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (test for excess errors)
gcc.c-torture/compile/complex-6.c   -O3 -g  (test for excess errors)
gcc.c-torture/compile/complex-6.c   -Os  (test for excess errors)
gcc.c-torture/compile/pr22422.c   -O1  (test for excess errors)
gcc.c-torture/compile/pr22422.c   -O2  (test for excess errors)
gcc.c-torture/compile/pr22422.c   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (test for excess errors)
gcc.c-torture/compile/pr22422.c   -O3 -g  (test for excess errors)
gcc.c-torture/compile/pr22422.c   -Os  (test for excess errors)
gcc.dg/ipa/ipcp-cstagg-5.c (test for excess errors)
gcc.dg/ipa/ipcp-cstagg-6.c (test for excess errors)
gcc.dg/torture/pr39074.c   -Os  (test for excess errors)
gcc.dg/torture/pr55882.c   -O1  (test for excess errors)
gcc.dg/torture/pr55882.c   -O2  (test for excess errors)
gcc.dg/torture/pr55882.c   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (test for excess errors)
gcc.dg/torture/pr55882.c   -O3 -g  (test for excess errors)
gcc.dg/torture/pr55882.c   -Os  (test for excess errors)
gcc.dg/torture/pr57569.c   -O1  (test for excess errors)
gcc.dg/torture/pr57569.c   -O2  (test for excess errors)
gcc.dg/torture/pr57569.c   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (test for excess errors)
gcc.dg/torture/pr57569.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  (test for excess errors)
gcc.dg/torture/pr57569.c   -Os  (test for excess errors)
gcc.dg/tree-ssa/ssa-fre-13.c (test for excess errors)
gcc.dg/tree-ssa/ssa-fre-42.c (test for excess errors)

An example ICEs are:
/home/jenkins/workspace/BuildThunderX_native_gcc_upstream/gcc/gcc/testsuite/gcc.c-torture/compile/20020604-1.c:91:1:
error: insn does not satisfy its constraints:
(insn 112 111 113 5 (set (reg:DI 7 x7 [196])
        (plus:DI (ashift:DI (reg:DI 7 x7 [194])
                (const_int 4 [0x4]))
            (reg/f:DI 31 sp))) 189 {*add_lsl_di}
     (nil))
/home/jenkins/workspace/BuildThunderX_native_gcc_upstream/gcc/gcc/testsuite/gcc.c-torture/compile/20020604-1.c:91:1:
internal compiler error: in extract_constrain_insn, at recog.c:2212

/home/jenkins/workspace/BuildThunderX_native_gcc_upstream/gcc/gcc/testsuite/gcc.c-torture/compile/pr22422.c:29:1:
error: insn does not satisfy its constraints:
(insn 7 6 12 2 (set (mem/f:DI (reg:DI 0 x0 [ b ]) [2 b_2(D)->a+0 S8 A64])
        (reg/f:DI 31 sp))
/home/jenkins/workspace/BuildThunderX_native_gcc_upstream/gcc/gcc/testsuite/gcc.c-torture/compile/pr22422.c:25
50 {*movdi_aarch64}
     (nil))


Notice how sp is being used there.  That register is not valid in that location.

Thanks,
Andrew

>
> Peter
>
>
>

Re: [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Peter Bergner]

On 9/9/16 5:51 PM, Jeff Law wrote:
> On 08/30/2016 10:23 PM, Peter Bergner wrote:
>> gcc/
>>     PR rtl-optimization/77289
>>     * lra-constraints.c (get_final_hard_regno): Add support for non hard
>>     register numbers.  Remove support for subregs.
>>     (get_hard_regno): Use SUBREG_P.  Don't call get_final_hard_regno().
>>     (get_reg_class): Delete removed get_final_hard_regno() argument.
>>     (uses_hard_regs_p): Call get_final_hard_regno().
>>
>> gcc/testsuite/
>>     PR rtl-optimization/77289
>>     * gcc.target/powerpc/pr77289.c: New test.
> OK.

Committed as revision 240065.  Thanks!

Peter

Re: [PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Jeff Law]

On 08/30/2016 10:23 PM, Peter Bergner wrote:
> PR77289 exposes a latent problem with LRA constraint matching.  In the buggy
> test cases, LRA performs a speculative register elimination before checking
> operands for matching constraints.  With the elimination, the operands
> appear to match.  However, when we call check_rtl() which attempts to
> recognize the instruction, the reg above it not eliminated leading to a
> "insn does not satisfy its constraints" ICE.
>
> This patch fixes the problem by not performing register elimination during
> constraint matching.  operands_match_p() uses get_hard_reg() to grab a
> REG's hard reg number, so I have removed get_hard_reg()'s call to
> get_final_hard_regno() which performs the register elimination, which
> fixes the bug.  uses_hard_regs_p() is the other caller of get_hard_regno()
> and it still needs register elimination to be called, so I have changed
> it to call get_final_hard_regno() instead.  Since uses_hard_regs_p()
> may call get_final_hard_regno() with a pseudo, I have added support
> for mapping those to hard reg numbers before performing the register
> elimination.
>
> This has passed bootstrap and regtesting with no regressions.
> Ok for mainline?
>
> Peter
>
> gcc/
> 	PR rtl-optimization/77289
> 	* lra-constraints.c (get_final_hard_regno): Add support for non hard
> 	register numbers.  Remove support for subregs.
> 	(get_hard_regno): Use SUBREG_P.  Don't call get_final_hard_regno().
> 	(get_reg_class): Delete removed get_final_hard_regno() argument.
> 	(uses_hard_regs_p): Call get_final_hard_regno().
>
> gcc/testsuite/
> 	PR rtl-optimization/77289
> 	* gcc.target/powerpc/pr77289.c: New test.
OK.

Jeff

[PATCH, LRA] Fix PR rtl-optimization 77289, LRA matching constraint problem

[Peter Bergner]

PR77289 exposes a latent problem with LRA constraint matching.  In the buggy
test cases, LRA performs a speculative register elimination before checking
operands for matching constraints.  With the elimination, the operands
appear to match.  However, when we call check_rtl() which attempts to
recognize the instruction, the reg above it not eliminated leading to a
"insn does not satisfy its constraints" ICE.

This patch fixes the problem by not performing register elimination during
constraint matching.  operands_match_p() uses get_hard_reg() to grab a
REG's hard reg number, so I have removed get_hard_reg()'s call to
get_final_hard_regno() which performs the register elimination, which
fixes the bug.  uses_hard_regs_p() is the other caller of get_hard_regno()
and it still needs register elimination to be called, so I have changed
it to call get_final_hard_regno() instead.  Since uses_hard_regs_p()
may call get_final_hard_regno() with a pseudo, I have added support
for mapping those to hard reg numbers before performing the register
elimination.

This has passed bootstrap and regtesting with no regressions.
Ok for mainline?

Peter

gcc/
	PR rtl-optimization/77289
	* lra-constraints.c (get_final_hard_regno): Add support for non hard
	register numbers.  Remove support for subregs.
	(get_hard_regno): Use SUBREG_P.  Don't call get_final_hard_regno().
	(get_reg_class): Delete removed get_final_hard_regno() argument.
	(uses_hard_regs_p): Call get_final_hard_regno().

gcc/testsuite/
	PR rtl-optimization/77289
	* gcc.target/powerpc/pr77289.c: New test.

Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	(revision 239866)
+++ gcc/lra-constraints.c	(working copy)
@@ -182,21 +182,22 @@ get_try_hard_regno (int regno)
   return ira_class_hard_regs[rclass][0];
 }
 
-/* Return final hard regno (plus offset) which will be after
-   elimination.	 We do this for matching constraints because the final
-   hard regno could have a different class.  */
+/* Return the final hard regno which will be after elimination.
+   We do this because the final hard regno could have a different class.  */
 static int
-get_final_hard_regno (int hard_regno, int offset)
+get_final_hard_regno (int regno)
 {
-  if (hard_regno < 0)
-    return hard_regno;
-  hard_regno = lra_get_elimination_hard_regno (hard_regno);
-  return hard_regno + offset;
+  if (! HARD_REGISTER_NUM_P (regno))
+    regno = lra_get_regno_hard_regno (regno);
+  if (regno < 0)
+    return regno;
+  return lra_get_elimination_hard_regno (regno);
 }
 
-/* Return hard regno of X after removing subreg and making
-   elimination.  If X is not a register or subreg of register, return
-   -1.  For pseudo use its assignment.  */
+/* Return the hard regno of X after removing its subreg.  If X is not
+   a register or a subreg of a register, return -1.  If X is a pseudo,
+   use its assignment.  We do not process register eliminiations while
+   matching constraints.  See PR77289.  */
 static int
 get_hard_regno (rtx x)
 {
@@ -204,19 +205,19 @@ get_hard_regno (rtx x)
   int offset, hard_regno;
 
   reg = x;
-  if (GET_CODE (x) == SUBREG)
+  if (SUBREG_P (x))
     reg = SUBREG_REG (x);
   if (! REG_P (reg))
     return -1;
-  if ((hard_regno = REGNO (reg)) >= FIRST_PSEUDO_REGISTER)
+  if (! HARD_REGISTER_NUM_P (hard_regno = REGNO (reg)))
     hard_regno = lra_get_regno_hard_regno (hard_regno);
   if (hard_regno < 0)
     return -1;
   offset = 0;
-  if (GET_CODE (x) == SUBREG)
+  if (SUBREG_P (x))
     offset += subreg_regno_offset (hard_regno, GET_MODE (reg),
 				   SUBREG_BYTE (x),  GET_MODE (x));
-  return get_final_hard_regno (hard_regno, offset);
+  return hard_regno + offset;
 }
 
 /* If REGNO is a hard register or has been allocated a hard register,
@@ -232,7 +233,7 @@ get_reg_class (int regno)
     hard_regno = lra_get_regno_hard_regno (regno);
   if (hard_regno >= 0)
     {
-      hard_regno = get_final_hard_regno (hard_regno, 0);
+      hard_regno = get_final_hard_regno (hard_regno);
       return REGNO_REG_CLASS (hard_regno);
     }
   if (regno >= new_regno_start)
@@ -1712,7 +1713,7 @@ uses_hard_regs_p (rtx x, HARD_REG_SET se
 
   if (REG_P (x))
     {
-      x_hard_regno = get_hard_regno (x);
+      x_hard_regno = get_final_hard_regno (REGNO (x));
       return (x_hard_regno >= 0
 	      && overlaps_hard_reg_set_p (set, mode, x_hard_regno));
     }
Index: gcc/testsuite/gcc.target/powerpc/pr77289.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr77289.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr77289.c	(working copy)
@@ -0,0 +1,31 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
+/* { dg-options "-O3 -mcpu=power7 -funroll-loops -ffast-math -mlra -mupdate -fno-auto-inc-dec" } */
+
+/* PR 77289: LRA ICEs due to invalid constraint checking.  */
+
+void dummy0 (float *);
+float bar0 (float);
+void
+foo0 (long a, long b)
+{
+  float c[0];
+  b = 0;
+  for (; b < a; b++)
+    c[b] = bar0 (c[b]);
+  dummy0 (c);
+}
+
+void dummy1 (long *);
+long bar1 (long);
+void
+foo1 (long a, long b)
+{
+  long array[128];
+  long *c = array;
+  for (b=0; b < a; b++)
+    c[b] = bar1 (c[b]);
+  dummy1 (c);
+}