public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728
@ 2014-11-05 8:20 zhenqiang.chen at arm dot com
2014-11-05 8:35 ` [Bug tree-optimization/63743] " jakub at gcc dot gnu.org
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: zhenqiang.chen at arm dot com @ 2014-11-05 8:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
Bug ID: 63743
Summary: Thumb1: big regression for float operators by r216728
Product: gcc
Version: 5.0
Status: UNCONFIRMED
Severity: critical
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: zhenqiang.chen at arm dot com
Created attachment 33887
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33887&action=edit
test case
Root cause: the fold_stmt swaps the operands, which leads to register shuffle.
commit f619ecaed41d1487091098a0f4fdf4d6ed1fa379
Author: rguenth <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Mon Oct 27 11:30:23 2014 +0000
2014-10-27 Richard Biener <rguenther@suse.de>
* tree-ssa-forwprop.c: Include tree-cfgcleanup.h and tree-into-ssa.h.
(lattice): New global.
(fwprop_ssa_val): New function.
(fold_all_stmts): Likewise.
(pass_forwprop::execute): Finally fold all stmts.
* gcc.dg/tree-ssa/forwprop-6.c: Scan ccp1 dump instead.
* gcc.dg/strlenopt-8.c: Adjust and XFAIL for non_strict_align
target due to memcpy inline-expansion.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@216728
138bc75d-0d04-0410-961f-82ee72b054a4
A simplified case is attached.
Options: -mthumb -Os -mcpu=cortex-m0
Before the patch, tree codes like
_20 = _14 + _19;
_21 = _20 * x_13;
After the patch, tree codes like
_20 = _14 + _19;
_21 = x_13 * _20;
Without HARD fpu support, all operators will be changed to function calls. The
assemble codes change like:
Before the patch,
bl __aeabi_dadd
ldr r2, [sp]
ldr r3, [sp, #4]
/* r0, r1 are reused from the return values of the previous call. */
bl __aeabi_dmul
After the patch,
bl __aeabi_dadd
mov r2, r0
mov r3, r1
ldr r0, [sp]
ldr r1, [sp, #4]
bl __aeabi_dmul
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
@ 2014-11-05 8:35 ` jakub at gcc dot gnu.org
2014-11-05 9:40 ` zhenqiang.chen at arm dot com
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2014-11-05 8:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Were we swapping operands before? I mean, if you rewrite the testcase to swap
the * arguments in the source, did you get the same more efficient code in the
past?
In any case, this doesn't sound something that we should keep in mind in GIMPLE
passes, + and * are just commutative, how they are expanded is a matter of
expansion. So, either look for this during expansion (if a commutative
operation is being expanded using libcall (or is this emitted by the backend?),
see if one of the operands isn't result of immediately preceeding emitted call
and if it is, perhaps swap the order of arguments), or add some register
allocator smarts (add a way to mark library calls as commutative and allow RA
to swap arguments to them if beneficial).
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
2014-11-05 8:35 ` [Bug tree-optimization/63743] " jakub at gcc dot gnu.org
@ 2014-11-05 9:40 ` zhenqiang.chen at arm dot com
2014-11-05 9:51 ` jakub at gcc dot gnu.org
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: zhenqiang.chen at arm dot com @ 2014-11-05 9:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
--- Comment #2 from Zhenqiang Chen <zhenqiang.chen at arm dot com> ---
(In reply to Jakub Jelinek from comment #1)
> Were we swapping operands before? I mean, if you rewrite the testcase to
> swap the * arguments in the source, did you get the same more efficient code
> in the past?
Yes. I tried the test case:
double
test1 (double x, double y)
{
return x * (x + y);
}
double
test2 (double x, double y)
{
return (x + y) * x;
}
Without r216728, I got efficient codes for both functions. But with r216728, I
got inefficient codes for both functions.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
2014-11-05 8:35 ` [Bug tree-optimization/63743] " jakub at gcc dot gnu.org
2014-11-05 9:40 ` zhenqiang.chen at arm dot com
@ 2014-11-05 9:51 ` jakub at gcc dot gnu.org
2014-11-05 10:21 ` zhenqiang.chen at arm dot com
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2014-11-05 9:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Zhenqiang Chen from comment #2)
> (In reply to Jakub Jelinek from comment #1)
> > Were we swapping operands before? I mean, if you rewrite the testcase to
> > swap the * arguments in the source, did you get the same more efficient code
> > in the past?
>
> Yes. I tried the test case:
>
> double
> test1 (double x, double y)
> {
> return x * (x + y);
> }
> double
> test2 (double x, double y)
> {
> return (x + y) * x;
> }
>
> Without r216728, I got efficient codes for both functions. But with r216728,
> I got inefficient codes for both functions.
What about
double
test3 (double x, double y)
{
return (x + y) * (x - y);
}
? At least from quick looking at ppc -msoft-float -O2 -m32, I see the same
issue there, add called first, sub called second, and result of second returned
in the same registers as used for the first argument. So something to handle at
expansion or RA rather than in GIMPLE anyway IMHO.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
` (2 preceding siblings ...)
2014-11-05 9:51 ` jakub at gcc dot gnu.org
@ 2014-11-05 10:21 ` zhenqiang.chen at arm dot com
2014-11-05 12:34 ` rguenth at gcc dot gnu.org
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: zhenqiang.chen at arm dot com @ 2014-11-05 10:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
--- Comment #4 from Zhenqiang Chen <zhenqiang.chen at arm dot com> ---
(In reply to Jakub Jelinek from comment #3)
> (In reply to Zhenqiang Chen from comment #2)
> > (In reply to Jakub Jelinek from comment #1)
> > > Were we swapping operands before? I mean, if you rewrite the testcase to
> > > swap the * arguments in the source, did you get the same more efficient code
> > > in the past?
> >
> > Yes. I tried the test case:
> >
> > double
> > test1 (double x, double y)
> > {
> > return x * (x + y);
> > }
> > double
> > test2 (double x, double y)
> > {
> > return (x + y) * x;
> > }
> >
> > Without r216728, I got efficient codes for both functions. But with r216728,
> > I got inefficient codes for both functions.
>
> What about
> double
> test3 (double x, double y)
> {
> return (x + y) * (x - y);
> }
> ? At least from quick looking at ppc -msoft-float -O2 -m32, I see the same
> issue there, add called first, sub called second, and result of second
> returned in the same registers as used for the first argument. So something
> to handle at expansion or RA rather than in GIMPLE anyway IMHO.
Same issue for the case on Thumb1.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
` (3 preceding siblings ...)
2014-11-05 10:21 ` zhenqiang.chen at arm dot com
@ 2014-11-05 12:34 ` rguenth at gcc dot gnu.org
2014-12-26 13:54 ` ysrumyan at gmail dot com
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-11-05 12:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenth at gcc dot gnu.org
--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
This is just operand canonicalization according to tree_swap_operands which is
now consistently applied. It was a bug that it wasn't applied before (the
usual offender here is SSA rewriting that doesn't fold "changed" statements
nor re-canonicalizes).
You are just (un)lucky btw, try both
double
test1 (double x, double y)
{
double tem = x + y;
double tem2 = x;
return tem2 * tem;
}
and
double
test1 (double x, double y)
{
double tem = x + y;
double tem2 = x;
return tem * tem2;
}
there is nothing that forces ordering in the way you would prefer it. So as
Jakub says - this needs addressing in RTL expansion and/or TER and SSA
coalescing.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
` (4 preceding siblings ...)
2014-11-05 12:34 ` rguenth at gcc dot gnu.org
@ 2014-12-26 13:54 ` ysrumyan at gmail dot com
2015-01-15 14:56 ` rguenth at gcc dot gnu.org
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: ysrumyan at gmail dot com @ 2014-12-26 13:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
Yuri Rumyantsev <ysrumyan at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |ysrumyan at gmail dot com
--- Comment #6 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
We also noticed regression on x86 32-bit targets after this fix and prepared a
simple fix which cures it: swap operands of commutative operations is performed
if cost of second operand is higher than the first one (the operand cost is
number of statements required for producing it). It is done at expand phase
inside expand_gimple_basic_block.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
` (5 preceding siblings ...)
2014-12-26 13:54 ` ysrumyan at gmail dot com
@ 2015-01-15 14:56 ` rguenth at gcc dot gnu.org
2015-01-28 8:01 ` thopre01 at gcc dot gnu.org
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: rguenth at gcc dot gnu.org @ 2015-01-15 14:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |WAITING
Last reconfirmed| |2015-01-15
Ever confirmed|0 |1
--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Was it fixed now?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
` (6 preceding siblings ...)
2015-01-15 14:56 ` rguenth at gcc dot gnu.org
@ 2015-01-28 8:01 ` thopre01 at gcc dot gnu.org
2015-03-04 6:58 ` thopre01 at gcc dot gnu.org
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: thopre01 at gcc dot gnu.org @ 2015-01-28 8:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
Thomas Preud'homme <thopre01 at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |thopre01 at gcc dot gnu.org
--- Comment #8 from Thomas Preud'homme <thopre01 at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> Was it fixed now?
I can still reproduce it. I'll try to look into it next month (and will assign
it to myself when I do).
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
` (7 preceding siblings ...)
2015-01-28 8:01 ` thopre01 at gcc dot gnu.org
@ 2015-03-04 6:58 ` thopre01 at gcc dot gnu.org
2015-03-09 1:32 ` thopre01 at gcc dot gnu.org
2015-03-11 5:06 ` thopre01 at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: thopre01 at gcc dot gnu.org @ 2015-03-04 6:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
Thomas Preud'homme <thopre01 at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|WAITING |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |thopre01 at gcc dot gnu.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
` (8 preceding siblings ...)
2015-03-04 6:58 ` thopre01 at gcc dot gnu.org
@ 2015-03-09 1:32 ` thopre01 at gcc dot gnu.org
2015-03-11 5:06 ` thopre01 at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: thopre01 at gcc dot gnu.org @ 2015-03-09 1:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
--- Comment #9 from Thomas Preud'homme <thopre01 at gcc dot gnu.org> ---
Author: thopre01
Date: Mon Mar 9 01:31:42 2015
New Revision: 221276
URL: https://gcc.gnu.org/viewcvs?rev=221276&root=gcc&view=rev
Log:
2015-03-09 Thomas Preud'homme <thomas.preudhomme@arm.com>
gcc/
PR tree-optimization/63743
* cfgexpand.c (reorder_operands): Also reorder if only second operand
had its definition forwarded by TER.
gcc/testsuite/
PR tree-optimization/63743
* gcc.dg/pr63743.c: New test.
Added:
trunk/gcc/testsuite/gcc.dg/pr63743.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/cfgexpand.c
trunk/gcc/testsuite/ChangeLog
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/63743] Thumb1: big regression for float operators by r216728
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
` (9 preceding siblings ...)
2015-03-09 1:32 ` thopre01 at gcc dot gnu.org
@ 2015-03-11 5:06 ` thopre01 at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: thopre01 at gcc dot gnu.org @ 2015-03-11 5:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63743
Thomas Preud'homme <thopre01 at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Known to work| |5.0
Resolution|--- |FIXED
--- Comment #10 from Thomas Preud'homme <thopre01 at gcc dot gnu.org> ---
Regression solved on trunk.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-03-11 5:06 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-05 8:20 [Bug tree-optimization/63743] New: Thumb1: big regression for float operators by r216728 zhenqiang.chen at arm dot com
2014-11-05 8:35 ` [Bug tree-optimization/63743] " jakub at gcc dot gnu.org
2014-11-05 9:40 ` zhenqiang.chen at arm dot com
2014-11-05 9:51 ` jakub at gcc dot gnu.org
2014-11-05 10:21 ` zhenqiang.chen at arm dot com
2014-11-05 12:34 ` rguenth at gcc dot gnu.org
2014-12-26 13:54 ` ysrumyan at gmail dot com
2015-01-15 14:56 ` rguenth at gcc dot gnu.org
2015-01-28 8:01 ` thopre01 at gcc dot gnu.org
2015-03-04 6:58 ` thopre01 at gcc dot gnu.org
2015-03-09 1:32 ` thopre01 at gcc dot gnu.org
2015-03-11 5:06 ` thopre01 at gcc dot gnu.org
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).