[PATCH][4.9]Backport "Fix register corruption bug in ree"

[PATCH][4.9]Backport "Fix register corruption bug in ree"

[Renlin Li]

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

Hi all,

This is a backport patch for branch 4.9. You can find the original patch 
here: https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00356.html
And it has been commit on the trunk as r215205.

This fixes a few libstdc++-v3 test suite failures.
x86_64 bootstraps Okay, aarch64_be-none-elf libstdc++-v3 tested Okay.

Okay to commit on branch 4.9?

Regards,
Renlin Li

2015-02-06  Renlin Li  <renlin.li@arm.com>

     Backport from mainline
     2014-09-12  Wilco Dijkstra  <wilco.dijkstra@arm.com>

     * gcc/ree.c (combine_reaching_defs): Ensure inserted copy don't change
     the number of hard registers.

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

diff --git a/gcc/ree.c b/gcc/ree.c
index 67fc9c3..e9ff904 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -793,6 +793,14 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
       if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE)
 	return false;
 
+      enum machine_mode dst_mode = GET_MODE (SET_DEST (PATTERN (cand->insn)));
+      rtx src_reg = get_extended_src_reg (SET_SRC (PATTERN (cand->insn)));
+
+      /* Ensure the number of hard registers of the copy match.  */
+      if (HARD_REGNO_NREGS (REGNO (src_reg), dst_mode)
+	  != HARD_REGNO_NREGS (REGNO (src_reg), GET_MODE (src_reg)))
+	return false;
+
       /* There's only one reaching def.  */
       rtx def_insn = state->defs_list[0];
 
@@ -842,7 +850,7 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
       start_sequence ();
       rtx pat = PATTERN (cand->insn);
       rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
-                                 REGNO (XEXP (SET_SRC (pat), 0)));
+				 REGNO (get_extended_src_reg (SET_SRC (pat))));
       rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
                                  REGNO (SET_DEST (pat)));
       emit_move_insn (new_dst, new_src);

[PING][PATCH][4.9]Backport "Fix register corruption bug in ree"

[Renlin Li]

Ping~
Anybody has time to review it?


Regards,
Renlin Li

On 06/02/15 17:48, Renlin Li wrote:
> Hi all,
>
> This is a backport patch for branch 4.9. You can find the original 
> patch here: https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00356.html
> And it has been commit on the trunk as r215205.
>
> This fixes a few libstdc++-v3 test suite failures.
> x86_64 bootstraps Okay, aarch64_be-none-elf libstdc++-v3 tested Okay.
>
> Okay to commit on branch 4.9?
>
> Regards,
> Renlin Li
>
> 2015-02-06  Renlin Li  <renlin.li@arm.com>
>
>     Backport from mainline
>     2014-09-12  Wilco Dijkstra  <wilco.dijkstra@arm.com>
>
>     * gcc/ree.c (combine_reaching_defs): Ensure inserted copy don't 
> change
>     the number of hard registers.
>

Re: [PING][PATCH][4.9]Backport "Fix register corruption bug in ree"

[Renlin Li]

Ping~

Regards,
Renlin Li

On 16/04/15 10:09, wrote:
> Ping~
> Anybody has time to review it?
>
>
> Regards,
> Renlin Li
>
> On 06/02/15 17:48, Renlin Li wrote:
>> Hi all,
>>
>> This is a backport patch for branch 4.9. You can find the original=20
>> patch here:https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00356.html
>> And it has been commit on the trunk as r215205.
>>
>> This fixes a few libstdc++-v3 test suite failures.
>> x86_64 bootstraps Okay, aarch64_be-none-elf libstdc++-v3 tested Okay.
>>
>> Okay to commit on branch 4.9?
>>
>> Regards,
>> Renlin Li
>>
>> 2015-02-06  Renlin Li<renlin.li@arm.com>
>>
>>      Backport from mainline
>>      2014-09-12  Wilco Dijkstra<wilco.dijkstra@arm.com>
>>
>>      * gcc/ree.c (combine_reaching_defs): Ensure inserted copy don't=20
>> change
>>      the number of hard registers.
>>

Re: [PING][PATCH][4.9]Backport "Fix register corruption bug in ree"

[Renlin Li]

Ping ~

Can somebody review it? Thank you!

Regards,
Renlin Li

On 16/04/15 10:32, Renlin Li wrote:
> Ping~
>
> Regards,
> Renlin Li
>
> On 16/04/15 10:09, wrote:
>> Ping~
>> Anybody has time to review it?
>>
>>
>> Regards,
>> Renlin Li
>>
>> On 06/02/15 17:48, Renlin Li wrote:
>>> Hi all,
>>>
>>> This is a backport patch for branch 4.9. You can find the original=20
>>> patch here:https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00356.html
>>> And it has been commit on the trunk as r215205.
>>>
>>> This fixes a few libstdc++-v3 test suite failures.
>>> x86_64 bootstraps Okay, aarch64_be-none-elf libstdc++-v3 tested Okay.
>>>
>>> Okay to commit on branch 4.9?
>>>
>>> Regards,
>>> Renlin Li
>>>
>>> 2015-02-06  Renlin Li<renlin.li@arm.com>
>>>
>>>      Backport from mainline
>>>      2014-09-12  Wilco Dijkstra<wilco.dijkstra@arm.com>
>>>
>>>      * gcc/ree.c (combine_reaching_defs): Ensure inserted copy don't=20
>>> change
>>>      the number of hard registers.
>>>

Re: [PING][PATCH][4.9]Backport "Fix register corruption bug in ree"

[Ramana Radhakrishnan]

On Thu, Jun 4, 2015 at 2:16 PM, Renlin Li <renlin.li@arm.com> wrote:
> Ping ~
>
> Can somebody review it? Thank you!
>
> Regards,
> Renlin Li
>
>
> On 16/04/15 10:32, Renlin Li wrote:
>>
>> Ping~
>>
>> Regards,
>> Renlin Li
>>
>> On 16/04/15 10:09, wrote:
>>>
>>> Ping~
>>> Anybody has time to review it?
>>>
>>>
>>> Regards,
>>> Renlin Li
>>>
>>> On 06/02/15 17:48, Renlin Li wrote:
>>>>
>>>> Hi all,
>>>>
>>>> This is a backport patch for branch 4.9. You can find the original=20
>>>> patch here:https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00356.html
>>>> And it has been commit on the trunk as r215205.
>>>>
>>>> This fixes a few libstdc++-v3 test suite failures.
>>>> x86_64 bootstraps Okay, aarch64_be-none-elf libstdc++-v3 tested Okay.
>>>>
>>>> Okay to commit on branch 4.9?
>>>>
>>>> Regards,
>>>> Renlin Li
>>>>
>>>> 2015-02-06  Renlin Li<renlin.li@arm.com>
>>>>
>>>>      Backport from mainline
>>>>      2014-09-12  Wilco Dijkstra<wilco.dijkstra@arm.com>
>>>>
>>>>      * gcc/ree.c (combine_reaching_defs): Ensure inserted copy don't=20
>>>> change
>>>>      the number of hard registers.
>>>>
>

richi - an RM question -

Is this something that can be pulled back to GCC 4.9 branch assuming
testing still shows no regressions  - it breaks aarch64 be on GCC 4.9
...


regards
Ramana

Re: [PING][PATCH][4.9]Backport "Fix register corruption bug in ree"

[Richard Biener]

On October 29, 2015 4:37:08 PM GMT+01:00, Ramana Radhakrishnan <ramana.gcc@googlemail.com> wrote:
>On Thu, Jun 4, 2015 at 2:16 PM, Renlin Li <renlin.li@arm.com> wrote:
>> Ping ~
>>
>> Can somebody review it? Thank you!
>>
>> Regards,
>> Renlin Li
>>
>>
>> On 16/04/15 10:32, Renlin Li wrote:
>>>
>>> Ping~
>>>
>>> Regards,
>>> Renlin Li
>>>
>>> On 16/04/15 10:09, wrote:
>>>>
>>>> Ping~
>>>> Anybody has time to review it?
>>>>
>>>>
>>>> Regards,
>>>> Renlin Li
>>>>
>>>> On 06/02/15 17:48, Renlin Li wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> This is a backport patch for branch 4.9. You can find the
>original=20
>>>>> patch
>here:https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00356.html
>>>>> And it has been commit on the trunk as r215205.
>>>>>
>>>>> This fixes a few libstdc++-v3 test suite failures.
>>>>> x86_64 bootstraps Okay, aarch64_be-none-elf libstdc++-v3 tested
>Okay.
>>>>>
>>>>> Okay to commit on branch 4.9?
>>>>>
>>>>> Regards,
>>>>> Renlin Li
>>>>>
>>>>> 2015-02-06  Renlin Li<renlin.li@arm.com>
>>>>>
>>>>>      Backport from mainline
>>>>>      2014-09-12  Wilco Dijkstra<wilco.dijkstra@arm.com>
>>>>>
>>>>>      * gcc/ree.c (combine_reaching_defs): Ensure inserted copy
>don't=20
>>>>> change
>>>>>      the number of hard registers.
>>>>>
>>
>
>richi - an RM question -
>
>Is this something that can be pulled back to GCC 4.9 branch assuming
>testing still shows no regressions  - it breaks aarch64 be on GCC 4.9

If it is a regression or wrong-code fix its OK with a test case.

Richard.

>...
>
>
>regards
>Ramana

Re: [PING][PATCH][4.9]Backport "Fix register corruption bug in ree"

[Renlin Li]

Hi Richard,

I am trying to come up with a simple test case, but failed to do so.

I have isolated the function where bug is triggered. However, in order 
to make it a useful test case, the silently corrupted register should be 
used somewhere, and should affect the correctness of the program. I 
cannot create such an context.


libstdc++-v3/testsuite/tr1/8_c_compatibility/complex/50880.cc is a 
concreted example which will trigger this bug.

Here is a short description about why the test case failed:

wrong code-gen in ree pass:

(insn 279 278 531 2 (set (reg:TF 32 v0)
         (zero_extend:TF (reg:DI 19 x19 [orig:150 __x+8 ] [150]))) 
/libstdc++-v3/include/complex:861 691 {aarch64_movtflow_di}
      (nil))
(insn 531 279 275 2 (set (reg:TF 3 x3)
         (reg:TF 32 v0)) /libstdc++-v3/include/complex:861 -1
      (nil))
(insn 275 531 510 2 (set (reg:DI 23 x23 [orig:151 __y ] [151])
         (mem:DI (plus:DI (reg/v/f:DI 0 x0 [orig:104 __z ] [104])
                 (const_int 16 [0x10])) [9 MEM[(const long double 
&)__z_4(D) + 16]+0 S8 A128])) /libstdc++-v3/include/complex:859 34 
{*movdi_aarch64}
      (nil))
(insn 510 275 19 2 (set (zero_extract:TF (reg:TF 32 v0)
             (const_int 64 [0x40])
             (const_int 64 [0x40]))
         (zero_extend:TF (reg:DI 2 x2 [ __x+-8 ]))) 
/libstdc++-v3/include/complex:861 689 {aarch64_movtfhigh_di}
      (nil))


insn 531 clobbers x4 as well. That's exactly the same problem described 
in Wilco's email. The following is what the correct code should look 
like. It's generated after applying Wilco's fix.


(insn 279 278 275 2 (set (reg:DI 3 x3 [orig:185 __x ] [185])
         (reg:DI 19 x19 [orig:150 __x+8 ] [150])) 
/libstdc++-v3/include/complex:861 34 {*movdi_aarch64}
      (nil))
(insn 275 279 509 2 (set (reg:DI 23 x23 [orig:151 __y ] [151])
         (mem:DI (plus:DI (reg/v/f:DI 0 x0 [orig:104 __z ] [104])
                 (const_int 16 [0x10])) [9 MEM[(const long double 
&)__z_4(D) + 16]+0 S8 A128])) /libstdc++-v3/include/complex:859 34 
{*movdi_aarch64}
      (nil))
(insn 509 275 510 2 (set (reg:TF 32 v0)
         (zero_extend:TF (reg:DI 3 x3 [orig:185 __x ] [185]))) 
/libstdc++-v3/include/complex:861 691 {aarch64_movtflow_di}
      (nil))
(insn 510 509 19 2 (set (zero_extract:TF (reg:TF 32 v0)
             (const_int 64 [0x40])
             (const_int 64 [0x40]))
         (zero_extend:TF (reg:DI 2 x2 [ __x+-8 ]))) 
/libstdc++-v3/include/complex:861 689 {aarch64_movtfhigh_di}
      (nil))



Regards,
Renlin Li

On 29/10/15 16:33, Richard Biener wrote:
> On October 29, 2015 4:37:08 PM GMT+01:00, Ramana Radhakrishnan <ramana.gcc@googlemail.com> wrote:
>> On Thu, Jun 4, 2015 at 2:16 PM, Renlin Li <renlin.li@arm.com> wrote:
>>> Ping ~
>>>
>>> Can somebody review it? Thank you!
>>>
>>> Regards,
>>> Renlin Li
>>>
>>>
>>> On 16/04/15 10:32, Renlin Li wrote:
>>>> Ping~
>>>>
>>>> Regards,
>>>> Renlin Li
>>>>
>>>> On 16/04/15 10:09, wrote:
>>>>> Ping~
>>>>> Anybody has time to review it?
>>>>>
>>>>>
>>>>> Regards,
>>>>> Renlin Li
>>>>>
>>>>> On 06/02/15 17:48, Renlin Li wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> This is a backport patch for branch 4.9. You can find the
>> original=20
>>>>>> patch
>> here:https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00356.html
>>>>>> And it has been commit on the trunk as r215205.
>>>>>>
>>>>>> This fixes a few libstdc++-v3 test suite failures.
>>>>>> x86_64 bootstraps Okay, aarch64_be-none-elf libstdc++-v3 tested
>> Okay.
>>>>>> Okay to commit on branch 4.9?
>>>>>>
>>>>>> Regards,
>>>>>> Renlin Li
>>>>>>
>>>>>> 2015-02-06  Renlin Li<renlin.li@arm.com>
>>>>>>
>>>>>>       Backport from mainline
>>>>>>       2014-09-12  Wilco Dijkstra<wilco.dijkstra@arm.com>
>>>>>>
>>>>>>       * gcc/ree.c (combine_reaching_defs): Ensure inserted copy
>> don't=20
>>>>>> change
>>>>>>       the number of hard registers.
>>>>>>
>> richi - an RM question -
>>
>> Is this something that can be pulled back to GCC 4.9 branch assuming
>> testing still shows no regressions  - it breaks aarch64 be on GCC 4.9
> If it is a regression or wrong-code fix its OK with a test case.
>
> Richard.
>
>> ...
>>
>>
>> regards
>> Ramana
>

Re: [PING][PATCH][4.9]Backport "Fix register corruption bug in ree"

[Richard Biener]

On Fri, 6 Nov 2015, Renlin Li wrote:

> Hi Richard,
> 
> I am trying to come up with a simple test case, but failed to do so.
> 
> I have isolated the function where bug is triggered. However, in order to make
> it a useful test case, the silently corrupted register should be used
> somewhere, and should affect the correctness of the program. I cannot create
> such an context.
> 
> 
> libstdc++-v3/testsuite/tr1/8_c_compatibility/complex/50880.cc is a concreted
> example which will trigger this bug.

Well, if that testcase exposes the bug you already have a testcase.

Richard.

> Here is a short description about why the test case failed:
> 
> wrong code-gen in ree pass:
> 
> (insn 279 278 531 2 (set (reg:TF 32 v0)
>         (zero_extend:TF (reg:DI 19 x19 [orig:150 __x+8 ] [150])))
> /libstdc++-v3/include/complex:861 691 {aarch64_movtflow_di}
>      (nil))
> (insn 531 279 275 2 (set (reg:TF 3 x3)
>         (reg:TF 32 v0)) /libstdc++-v3/include/complex:861 -1
>      (nil))
> (insn 275 531 510 2 (set (reg:DI 23 x23 [orig:151 __y ] [151])
>         (mem:DI (plus:DI (reg/v/f:DI 0 x0 [orig:104 __z ] [104])
>                 (const_int 16 [0x10])) [9 MEM[(const long double &)__z_4(D) +
> 16]+0 S8 A128])) /libstdc++-v3/include/complex:859 34 {*movdi_aarch64}
>      (nil))
> (insn 510 275 19 2 (set (zero_extract:TF (reg:TF 32 v0)
>             (const_int 64 [0x40])
>             (const_int 64 [0x40]))
>         (zero_extend:TF (reg:DI 2 x2 [ __x+-8 ])))
> /libstdc++-v3/include/complex:861 689 {aarch64_movtfhigh_di}
>      (nil))
> 
> 
> insn 531 clobbers x4 as well. That's exactly the same problem described in
> Wilco's email. The following is what the correct code should look like. It's
> generated after applying Wilco's fix.
> 
> 
> (insn 279 278 275 2 (set (reg:DI 3 x3 [orig:185 __x ] [185])
>         (reg:DI 19 x19 [orig:150 __x+8 ] [150]))
> /libstdc++-v3/include/complex:861 34 {*movdi_aarch64}
>      (nil))
> (insn 275 279 509 2 (set (reg:DI 23 x23 [orig:151 __y ] [151])
>         (mem:DI (plus:DI (reg/v/f:DI 0 x0 [orig:104 __z ] [104])
>                 (const_int 16 [0x10])) [9 MEM[(const long double &)__z_4(D) +
> 16]+0 S8 A128])) /libstdc++-v3/include/complex:859 34 {*movdi_aarch64}
>      (nil))
> (insn 509 275 510 2 (set (reg:TF 32 v0)
>         (zero_extend:TF (reg:DI 3 x3 [orig:185 __x ] [185])))
> /libstdc++-v3/include/complex:861 691 {aarch64_movtflow_di}
>      (nil))
> (insn 510 509 19 2 (set (zero_extract:TF (reg:TF 32 v0)
>             (const_int 64 [0x40])
>             (const_int 64 [0x40]))
>         (zero_extend:TF (reg:DI 2 x2 [ __x+-8 ])))
> /libstdc++-v3/include/complex:861 689 {aarch64_movtfhigh_di}
>      (nil))
> 
> 
> 
> Regards,
> Renlin Li
> 
> On 29/10/15 16:33, Richard Biener wrote:
> > On October 29, 2015 4:37:08 PM GMT+01:00, Ramana Radhakrishnan
> > <ramana.gcc@googlemail.com> wrote:
> > > On Thu, Jun 4, 2015 at 2:16 PM, Renlin Li <renlin.li@arm.com> wrote:
> > > > Ping ~
> > > > 
> > > > Can somebody review it? Thank you!
> > > > 
> > > > Regards,
> > > > Renlin Li
> > > > 
> > > > 
> > > > On 16/04/15 10:32, Renlin Li wrote:
> > > > > Ping~
> > > > > 
> > > > > Regards,
> > > > > Renlin Li
> > > > > 
> > > > > On 16/04/15 10:09, wrote:
> > > > > > Ping~
> > > > > > Anybody has time to review it?
> > > > > > 
> > > > > > 
> > > > > > Regards,
> > > > > > Renlin Li
> > > > > > 
> > > > > > On 06/02/15 17:48, Renlin Li wrote:
> > > > > > > Hi all,
> > > > > > > 
> > > > > > > This is a backport patch for branch 4.9. You can find the
> > > original=20
> > > > > > > patch
> > > here:https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00356.html
> > > > > > > And it has been commit on the trunk as r215205.
> > > > > > > 
> > > > > > > This fixes a few libstdc++-v3 test suite failures.
> > > > > > > x86_64 bootstraps Okay, aarch64_be-none-elf libstdc++-v3 tested
> > > Okay.
> > > > > > > Okay to commit on branch 4.9?
> > > > > > > 
> > > > > > > Regards,
> > > > > > > Renlin Li
> > > > > > > 
> > > > > > > 2015-02-06  Renlin Li<renlin.li@arm.com>
> > > > > > > 
> > > > > > >       Backport from mainline
> > > > > > >       2014-09-12  Wilco Dijkstra<wilco.dijkstra@arm.com>
> > > > > > > 
> > > > > > >       * gcc/ree.c (combine_reaching_defs): Ensure inserted copy
> > > don't=20
> > > > > > > change
> > > > > > >       the number of hard registers.
> > > > > > > 
> > > richi - an RM question -
> > > 
> > > Is this something that can be pulled back to GCC 4.9 branch assuming
> > > testing still shows no regressions  - it breaks aarch64 be on GCC 4.9
> > If it is a regression or wrong-code fix its OK with a test case.
> > 
> > Richard.
> > 
> > > ...
> > > 
> > > 
> > > regards
> > > Ramana
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)