optimization/10287: [3.2/3.4 regression] Loop/conditional store bug (ARM)

optimization/10287: [3.2/3.4 regression] Loop/conditional store bug (ARM)

[drow]

>Number:         10287
>Category:       optimization
>Synopsis:       [3.2/3.4 regression] Loop/conditional store bug (ARM)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    unassigned
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Apr 01 18:46:00 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     drow@mvista.com
>Release:        3.2.2 (release), 3.4 CVS (2003-03-27)
>Organization:
>Environment:
i686-pc-linux-gnu host, arm-linux target (or arm-elf, xscale-elf, et cetera)
>Description:
The attached test case aborts when compiled with -O2 for
an ARM target.

The diff between failing and succeeding binaries is:
        cmp     ip, #0
-       moveq   r3, r1
-       movne   r3, lr
        str     r1, [r5], #4
+       strne   lr, [r5], #4
        subs    ip, ip, #2
-       str     r3, [r5], #4

Note that the failing binary has an unconditional store
of a conditional value.  My guess is that something is
not respecting post-increment when distinguishing between
the two stores.


This is a 3.2 and 3.4 regression; it worked in 2.95 and
it works in 3.3.  The bug was fixed in 3.3 and HEAD by:

 2002-09-18  Richard Henderson  <rth@redhat.com>
 
       * ifcvt.c (noce_process_if_block): Correctly detect X modified
       with INSN_B before COND_EARLIEST.  Don't check A and B for 
       modification in condition range.  Reorder INSN_B for A==B properly.
       (if_convert): Iterate until no matches for a block.

It was then broken again in HEAD by:
Wed Jan  8 12:10:57 CET 2003  Jan Hubicka  <jh@suse.cz>

       * i386.md (adddi3_carry_rex64, subdi3_carry_rex64): Name pattern.
       (addhi3_carry, addqi3_carry, subhi3_carry, subqi3_carry): New patterns.
       (add??cc): New expanders.
       * i386.c (expand_int_addcc): New function.
       * i386-protos.h (expand_int_addcc): Declare.

       * alias.c (memory_modified_1): New static function.
       (memory_modified): New static varaible.
       (memory_modified_in_insn_p): New global function.
       * rtl.h (memory_modified_in_insn_p): Declare.
       * rtlanal.c (modified_between_p, modified_in_p): Be smart about memory
       references.

       * expr.h (emit_conditional_add): Declare.
>How-To-Repeat:

>Fix:

>Release-Note:
>Audit-Trail:
>Unformatted:
----gnatsweb-attachment----
Content-Type: text/x-csrc; name="btest.c"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="btest.c"

aW50IG1haW4oKQp7CglpbnQgcERzdFsxNl07CglpbnQgaSwgbkxlbmd0aCwgKnBEc3QxOwoKCWZv
ciAoaT0wOyBpPDE2OyBpKyspCgkJcERzdFtpXSA9IDA7CgoJbkxlbmd0aCA9IDQ7CglwRHN0MSA9
IHBEc3Q7Cglmb3IgKGk9KG5MZW5ndGgtMik7IGk+PTA7IGktPTIpCgl7CgkJKnBEc3QxKysgPSAx
OwoJCWlmKGkpCgkJCSpwRHN0MSsrID0gMjsKCX0KCglpZiAocERzdFszXSAhPSAwKQoJCWFib3J0
ICgpOwoJcmV0dXJuIDA7Cn0K

Re: optimization/10287: [3.2/3.4 regression] Loop/conditional store bug (ARM)

[Jan Hubicka]

The following reply was made to PR optimization/10287; it has been noted by GNATS.

From: Jan Hubicka <jh@suse.cz>
To: gcc-gnats@gcc.gnu.org, jh@suse.cz
Cc:  
Subject: Re: optimization/10287: [3.2/3.4 regression] Loop/conditional store bug (ARM)
Date: Wed, 2 Apr 2003 11:09:26 +0200

 > On Tue, Apr 01, 2003 at 06:36:11PM -0000, drow@mvista.com wrote:
 > > 
 > > >Number:         10287
 > > >Category:       optimization
 > > >Synopsis:       [3.2/3.4 regression] Loop/conditional store bug (ARM)
 > > >Confidential:   no
 > > >Severity:       serious
 > > >Priority:       medium
 > > >Responsible:    unassigned
 > > >State:          open
 > > >Class:          sw-bug
 > > >Submitter-Id:   net
 > > >Arrival-Date:   Tue Apr 01 18:46:00 UTC 2003
 > > >Closed-Date:
 > > >Last-Modified:
 > > >Originator:     drow@mvista.com
 > > >Release:        3.2.2 (release), 3.4 CVS (2003-03-27)
 > > >Organization:
 > > >Environment:
 > > i686-pc-linux-gnu host, arm-linux target (or arm-elf, xscale-elf, et cetera)
 > > >Description:
 > > The attached test case aborts when compiled with -O2 for
 > > an ARM target.
 > > 
 > > The diff between failing and succeeding binaries is:
 > >         cmp     ip, #0
 > > -       moveq   r3, r1
 > > -       movne   r3, lr
 > >         str     r1, [r5], #4
 > > +       strne   lr, [r5], #4
 > >         subs    ip, ip, #2
 > > -       str     r3, [r5], #4
 > > 
 > > Note that the failing binary has an unconditional store
 > > of a conditional value.  My guess is that something is
 > > not respecting post-increment when distinguishing between
 > > the two stores.
 > > 
 > > 
 > > This is a 3.2 and 3.4 regression; it worked in 2.95 and
 > > it works in 3.3.  The bug was fixed in 3.3 and HEAD by:
 > > 
 > >  2002-09-18  Richard Henderson  <rth@redhat.com>
 > >  
 > >        * ifcvt.c (noce_process_if_block): Correctly detect X modified
 > >        with INSN_B before COND_EARLIEST.  Don't check A and B for 
 > >        modification in condition range.  Reorder INSN_B for A==B properly.
 > >        (if_convert): Iterate until no matches for a block.
 > > 
 > > It was then broken again in HEAD by:
 > > Wed Jan  8 12:10:57 CET 2003  Jan Hubicka  <jh@suse.cz>
 > > 
 > >        * i386.md (adddi3_carry_rex64, subdi3_carry_rex64): Name pattern.
 > >        (addhi3_carry, addqi3_carry, subhi3_carry, subqi3_carry): New patterns.
 > >        (add??cc): New expanders.
 > >        * i386.c (expand_int_addcc): New function.
 > >        * i386-protos.h (expand_int_addcc): Declare.
 > > 
 > >        * alias.c (memory_modified_1): New static function.
 > >        (memory_modified): New static varaible.
 > >        (memory_modified_in_insn_p): New global function.
 > >        * rtl.h (memory_modified_in_insn_p): Declare.
 > >        * rtlanal.c (modified_between_p, modified_in_p): Be smart about memory
 > >        references.
 > > 
 > >        * expr.h (emit_conditional_add): Declare.
 > 
 > I'm not sure, but I think that modified_between_p and modified_in_p are
 > going to have to have POST_INC (POST_DEC, PRE_INC, PRE_DEC) cases in
 > them for the above patch from Jan to be safe.
 
 Hmm, possibly.  Is the "#4" enconding of auto increment?
 There is code to deal with the auto increments in alias.c and I think it
 is enought in this particular case.  WIll try to dig into it.
 
 Honza
 > 
 > 
 > -- 
 > Daniel Jacobowitz
 > MontaVista Software                         Debian GNU/Linux Developer

Re: optimization/10287: [3.2/3.4 regression] Loop/conditional store bug (ARM)

[Daniel Jacobowitz]

The following reply was made to PR optimization/10287; it has been noted by GNATS.

From: Daniel Jacobowitz <drow@mvista.com>
To: gcc-gnats@gcc.gnu.org
Cc: jh@suse.cz
Subject: Re: optimization/10287: [3.2/3.4 regression] Loop/conditional store bug (ARM)
Date: Tue, 1 Apr 2003 14:56:17 -0500

 On Tue, Apr 01, 2003 at 06:36:11PM -0000, drow@mvista.com wrote:
 > 
 > >Number:         10287
 > >Category:       optimization
 > >Synopsis:       [3.2/3.4 regression] Loop/conditional store bug (ARM)
 > >Confidential:   no
 > >Severity:       serious
 > >Priority:       medium
 > >Responsible:    unassigned
 > >State:          open
 > >Class:          sw-bug
 > >Submitter-Id:   net
 > >Arrival-Date:   Tue Apr 01 18:46:00 UTC 2003
 > >Closed-Date:
 > >Last-Modified:
 > >Originator:     drow@mvista.com
 > >Release:        3.2.2 (release), 3.4 CVS (2003-03-27)
 > >Organization:
 > >Environment:
 > i686-pc-linux-gnu host, arm-linux target (or arm-elf, xscale-elf, et cetera)
 > >Description:
 > The attached test case aborts when compiled with -O2 for
 > an ARM target.
 > 
 > The diff between failing and succeeding binaries is:
 >         cmp     ip, #0
 > -       moveq   r3, r1
 > -       movne   r3, lr
 >         str     r1, [r5], #4
 > +       strne   lr, [r5], #4
 >         subs    ip, ip, #2
 > -       str     r3, [r5], #4
 > 
 > Note that the failing binary has an unconditional store
 > of a conditional value.  My guess is that something is
 > not respecting post-increment when distinguishing between
 > the two stores.
 > 
 > 
 > This is a 3.2 and 3.4 regression; it worked in 2.95 and
 > it works in 3.3.  The bug was fixed in 3.3 and HEAD by:
 > 
 >  2002-09-18  Richard Henderson  <rth@redhat.com>
 >  
 >        * ifcvt.c (noce_process_if_block): Correctly detect X modified
 >        with INSN_B before COND_EARLIEST.  Don't check A and B for 
 >        modification in condition range.  Reorder INSN_B for A==B properly.
 >        (if_convert): Iterate until no matches for a block.
 > 
 > It was then broken again in HEAD by:
 > Wed Jan  8 12:10:57 CET 2003  Jan Hubicka  <jh@suse.cz>
 > 
 >        * i386.md (adddi3_carry_rex64, subdi3_carry_rex64): Name pattern.
 >        (addhi3_carry, addqi3_carry, subhi3_carry, subqi3_carry): New patterns.
 >        (add??cc): New expanders.
 >        * i386.c (expand_int_addcc): New function.
 >        * i386-protos.h (expand_int_addcc): Declare.
 > 
 >        * alias.c (memory_modified_1): New static function.
 >        (memory_modified): New static varaible.
 >        (memory_modified_in_insn_p): New global function.
 >        * rtl.h (memory_modified_in_insn_p): Declare.
 >        * rtlanal.c (modified_between_p, modified_in_p): Be smart about memory
 >        references.
 > 
 >        * expr.h (emit_conditional_add): Declare.
 
 I'm not sure, but I think that modified_between_p and modified_in_p are
 going to have to have POST_INC (POST_DEC, PRE_INC, PRE_DEC) cases in
 them for the above patch from Jan to be safe.
 
 
 -- 
 Daniel Jacobowitz
 MontaVista Software                         Debian GNU/Linux Developer