public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/40835] New: redundant comparison instruction
@ 2009-07-23 8:38 carrot at google dot com
2009-07-23 8:38 ` [Bug target/40835] " carrot at google dot com
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: carrot at google dot com @ 2009-07-23 8:38 UTC (permalink / raw)
To: gcc-bugs
Compile the following code with options -Os -mthumb -march=armv5te
int bar();
void goo(int, int);
void foo()
{
int v = bar();
if (v == 0)
return;
goo(1, v);
}
Gcc generates:
push {r3, lr}
bl bar
mov r1, r0
cmp r0, #0 // *
beq .L1
mov r0, #1
bl goo
.L1:
@ sp needed for prologue
pop {r3, pc}
The compare instruction is redundant since the previous move instruction has
already set the condition code according to the value of r0.
--
Summary: redundant comparison instruction
Product: gcc
Version: 4.5.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
AssignedTo: unassigned at gcc dot gnu dot org
ReportedBy: carrot at google dot com
GCC build triplet: i686-linux
GCC host triplet: i686-linux
GCC target triplet: arm-eabi
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
@ 2009-07-23 8:38 ` carrot at google dot com
2009-07-24 2:11 ` carrot at google dot com
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: carrot at google dot com @ 2009-07-23 8:38 UTC (permalink / raw)
To: gcc-bugs
------- Comment #1 from carrot at google dot com 2009-07-23 08:38 -------
Created an attachment (id=18241)
--> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=18241&action=view)
test case
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
2009-07-23 8:38 ` [Bug target/40835] " carrot at google dot com
@ 2009-07-24 2:11 ` carrot at google dot com
2009-07-24 7:00 ` steven at gcc dot gnu dot org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: carrot at google dot com @ 2009-07-24 2:11 UTC (permalink / raw)
To: gcc-bugs
------- Comment #2 from carrot at google dot com 2009-07-24 02:11 -------
It seems HAVE_cc0 disabled for arm. What's the reason behind it?
A simple method is to add a peephole rule to handle it.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
2009-07-23 8:38 ` [Bug target/40835] " carrot at google dot com
2009-07-24 2:11 ` carrot at google dot com
@ 2009-07-24 7:00 ` steven at gcc dot gnu dot org
2009-07-24 7:38 ` carrot at google dot com
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: steven at gcc dot gnu dot org @ 2009-07-24 7:00 UTC (permalink / raw)
To: gcc-bugs
------- Comment #3 from steven at gcc dot gnu dot org 2009-07-24 06:59 -------
Because HAVE_cc0 is only for cc0 targets, and ARM is not one of those?
You should stop jumping to peepholes for every missed optimization you find.
There is a csecc pass (part of cse2) that should handle this. You should try to
figure out why that pass can't optimize away your redundant insn.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
` (2 preceding siblings ...)
2009-07-24 7:00 ` steven at gcc dot gnu dot org
@ 2009-07-24 7:38 ` carrot at google dot com
2009-07-24 8:25 ` steven at gcc dot gnu dot org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: carrot at google dot com @ 2009-07-24 7:38 UTC (permalink / raw)
To: gcc-bugs
------- Comment #4 from carrot at google dot com 2009-07-24 07:37 -------
Just as I've figured out HAVE_cc0 is disabled. And cse_condition_code_reg does
nothing for thumb target.
I also found that the conditional branch instructions is always in the same
insn pattern as the previous compare instructions. So I even wonder there is
any way to express the optimized sequence (movs followed by bcc).
Is there any other places that I should take a look?
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
` (3 preceding siblings ...)
2009-07-24 7:38 ` carrot at google dot com
@ 2009-07-24 8:25 ` steven at gcc dot gnu dot org
2009-07-24 8:49 ` steven at gcc dot gnu dot org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: steven at gcc dot gnu dot org @ 2009-07-24 8:25 UTC (permalink / raw)
To: gcc-bugs
------- Comment #5 from steven at gcc dot gnu dot org 2009-07-24 08:25 -------
The fact that the move sets the condition code is not modelled in the insn.
>From .expand dump:
(insn 6 5 7 3 t.c:5 (set (reg/v:SI 133 [ v ])
(reg:SI 0 r0)) -1 (nil))
>From -dAP output:
@(insn 6 5 7 t.c:5 (set (reg/v:SI 1 r1 [orig:133 v ] [133])
@ (reg:SI 0 r0)) 167 {*thumb1_movsi_insn} (nil))
@ 0x0004
mov r1, r0 @ 6 *thumb1_movsi_insn/1 [length = 2]
So the compiler never even has a chance.
You could model a new pattern and let combine figure out that the move&compare
can be one insn. Or you can indeed do a peephole. The problem with peepholes
is that you may miss opportunities, but adding new patterns also has its
downsides...
--
steven at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Ever Confirmed|0 |1
Last reconfirmed|0000-00-00 00:00:00 |2009-07-24 08:25:30
date| |
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
` (4 preceding siblings ...)
2009-07-24 8:25 ` steven at gcc dot gnu dot org
@ 2009-07-24 8:49 ` steven at gcc dot gnu dot org
2009-07-24 14:09 ` rearnsha at gcc dot gnu dot org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: steven at gcc dot gnu dot org @ 2009-07-24 8:49 UTC (permalink / raw)
To: gcc-bugs
------- Comment #6 from steven at gcc dot gnu dot org 2009-07-24 08:48 -------
In fact even the compare isn't a separate insn:
@(insn 6 5 7 t.c:5 (set (reg/v:SI 1 r1 [orig:133 v ] [133])
@ (reg:SI 0 r0)) 167 {*thumb1_movsi_insn} (nil))
@ 0x0004
mov r1, r0 @ 6 *thumb1_movsi_insn/1 [length = 2]
@(jump_insn 7 6 8 t.c:6 (set (pc)
@ (if_then_else (eq (reg:SI 0 r0 [orig:133 v ] [133])
@ (const_int 0 [0x0]))
@ (label_ref:SI 14)
@ (pc))) 201 {*cbranchsi4_insn} (expr_list:REG_DEAD (reg:SI 0 r0
[orig:133 v ] [133])
@ (expr_list:REG_BR_PROB (const_int 6102 [0x17d6])
@ (nil))))
@ 0x0006
cmp r0, #0 @ 7 *cbranchsi4_insn/1 [length = 4]
beq .L1
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
` (5 preceding siblings ...)
2009-07-24 8:49 ` steven at gcc dot gnu dot org
@ 2009-07-24 14:09 ` rearnsha at gcc dot gnu dot org
2009-11-04 14:10 ` rearnsha at gcc dot gnu dot org
2009-11-23 8:51 ` carrot at google dot com
8 siblings, 0 replies; 10+ messages in thread
From: rearnsha at gcc dot gnu dot org @ 2009-07-24 14:09 UTC (permalink / raw)
To: gcc-bugs
------- Comment #7 from rearnsha at gcc dot gnu dot org 2009-07-24 14:08 -------
(In reply to comment #6)
> In fact even the compare isn't a separate insn:
There's a reason for that.
If you separate compares from uses of the flags then reload may end up
inserting add or mov instructions in between the comparison and its use. Since
thumb1 does not have non-flag setting versions of those instructions (at least
for the lo-regs case) we thus cannot separate the two.
To mitigate this, there are already a number of patterns in the thumb
description that try to exploit flag-setting instructions more efficiently, but
there are bound to be more cases that are not yet handled. Beware, however, of
generating sequences that are impossible to reload.
--
rearnsha at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rearnsha at gcc dot gnu dot
| |org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
` (6 preceding siblings ...)
2009-07-24 14:09 ` rearnsha at gcc dot gnu dot org
@ 2009-11-04 14:10 ` rearnsha at gcc dot gnu dot org
2009-11-23 8:51 ` carrot at google dot com
8 siblings, 0 replies; 10+ messages in thread
From: rearnsha at gcc dot gnu dot org @ 2009-11-04 14:10 UTC (permalink / raw)
To: gcc-bugs
------- Comment #8 from rearnsha at gcc dot gnu dot org 2009-11-04 14:10 -------
Subject: Bug 40835
Author: rearnsha
Date: Wed Nov 4 14:09:55 2009
New Revision: 153895
URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153895
Log:
2009-11-04 Richard Earnshaw <rearnsha@arm.com>
PR target/40835
* arm.md (peephole2 patterns for move and compare): New.
2009-11-04 Wei Guozhi <carrot@google.com>
PR target/40835
* gcc.target/arm/pr40835: New testcase.
Added:
trunk/gcc/testsuite/gcc.target/arm/pr40835.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm.md
trunk/gcc/testsuite/ChangeLog
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug target/40835] redundant comparison instruction
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
` (7 preceding siblings ...)
2009-11-04 14:10 ` rearnsha at gcc dot gnu dot org
@ 2009-11-23 8:51 ` carrot at google dot com
8 siblings, 0 replies; 10+ messages in thread
From: carrot at google dot com @ 2009-11-23 8:51 UTC (permalink / raw)
To: gcc-bugs
------- Comment #9 from carrot at google dot com 2009-11-23 08:51 -------
Fixed by Richard. Close it.
--
carrot at google dot com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40835
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-11-23 8:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-23 8:38 [Bug target/40835] New: redundant comparison instruction carrot at google dot com
2009-07-23 8:38 ` [Bug target/40835] " carrot at google dot com
2009-07-24 2:11 ` carrot at google dot com
2009-07-24 7:00 ` steven at gcc dot gnu dot org
2009-07-24 7:38 ` carrot at google dot com
2009-07-24 8:25 ` steven at gcc dot gnu dot org
2009-07-24 8:49 ` steven at gcc dot gnu dot org
2009-07-24 14:09 ` rearnsha at gcc dot gnu dot org
2009-11-04 14:10 ` rearnsha at gcc dot gnu dot org
2009-11-23 8:51 ` carrot at google dot com
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).