public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/49468] New: SH Target: inefficient integer abs code
@ 2011-06-19 15:15 oleg.endo@t-online.de
2011-06-19 15:30 ` [Bug target/49468] " oleg.endo@t-online.de
` (11 more replies)
0 siblings, 12 replies; 13+ messages in thread
From: oleg.endo@t-online.de @ 2011-06-19 15:15 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
Summary: SH Target: inefficient integer abs code
Product: gcc
Version: 4.7.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
AssignedTo: unassigned@gcc.gnu.org
ReportedBy: oleg.endo@t-online.de
The generated code for abs:SI and abs:DI is a bit inefficient:
int abs (int i)
{
return (i < 0) ? -i : i;
}
mov r4,r1
shll r1
subc r1,r1
mov r1,r0
xor r4,r0
rts
sub r1,r0
long long abs (long long i)
{
return (i < 0) ? -i : i;
}
mov r4,r3
shll r3
subc r3,r3
mov r5,r1
xor r3,r1
mov r3,r0
clrt
xor r4,r0
subc r3,r1
rts
subc r3,r0
There is a define_split in sh.md which is supposed to handle the special case
for SH4 but it is not doing anything. The problem has been around since a
couple of GCC 4.x versions.
sh-elf-gcc -v
Using built-in specs.
COLLECT_GCC=sh-elf-gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/sh-elf/4.7.0/lto-wrapper
Target: sh-elf
Configured with: ../gcc-trunk/configure --target=sh-elf --prefix=/usr/local
--enable-languages=c,c++ --enable-multilib --disable-libssp --without-headers
--disable-nls --disable-werror --enable-lto --with-newlib --with-gnu-as
--with-gnu-ld --with-system-zlib : (reconfigured) ../gcc-trunk/configure
--target=sh-elf --prefix=/usr/local --enable-languages=c,c++ --enable-multilib
--disable-libssp --without-headers --disable-nls --disable-werror --enable-lto
--with-newlib --with-gnu-as --with-gnu-ld --with-system-zlib : (reconfigured)
../gcc-trunk/configure --target=sh-elf --prefix=/usr/local
--enable-languages=c,c++ --enable-multilib --disable-libssp --without-headers
--disable-nls --disable-werror --enable-lto --with-newlib --with-gnu-as
--with-gnu-ld --with-system-zlib : (reconfigured) ../gcc-trunk/configure
--target=sh-elf --prefix=/usr/local --enable-multilib --disable-libssp
--without-headers --disable-nls --disable-werror --enable-lto --with-newlib
--with-gnu-as --with-gnu-ld --with-system-zlib target_alias=sh-elf CFLAGS=-Os
CXXFLAGS=-Os --enable-languages=c,c++,lto --no-create --no-recursion
Thread model: single
gcc version 4.7.0 20110619 (experimental) (GCC)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
@ 2011-06-19 15:30 ` oleg.endo@t-online.de
2011-06-19 15:49 ` oleg.endo@t-online.de
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: oleg.endo@t-online.de @ 2011-06-19 15:30 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
--- Comment #1 from Oleg Endo <oleg.endo@t-online.de> 2011-06-19 15:29:45 UTC ---
Created attachment 24560
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24560
Proposed patch
The patch adds explicit handling of abs:SI and abs:DI to the machine
description instead of relying on the default abs handling. On SH4 the
zero-offset branch way is definitely faster. It reduces pressure on EX group
instructions and improves parallel instruction execution. On other than SH4
zero-offset branches are not as fast, but the resulting code should still be
faster than the default branch-free abs code, at least it is more compact.
The patch also handles the case of neg (abs (...)) by simply inverting the
branch condition.
Feedback appreciated.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
2011-06-19 15:30 ` [Bug target/49468] " oleg.endo@t-online.de
@ 2011-06-19 15:49 ` oleg.endo@t-online.de
2011-06-22 22:38 ` kkojima at gcc dot gnu.org
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: oleg.endo@t-online.de @ 2011-06-19 15:49 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
--- Comment #2 from Oleg Endo <oleg.endo@t-online.de> 2011-06-19 15:48:41 UTC ---
Created attachment 24561
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24561
Before/After Examples
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
2011-06-19 15:30 ` [Bug target/49468] " oleg.endo@t-online.de
2011-06-19 15:49 ` oleg.endo@t-online.de
@ 2011-06-22 22:38 ` kkojima at gcc dot gnu.org
2011-06-26 20:49 ` oleg.endo@t-online.de
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: kkojima at gcc dot gnu.org @ 2011-06-22 22:38 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
Kazumoto Kojima <kkojima at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|normal |enhancement
--- Comment #3 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-06-22 22:37:28 UTC ---
On sh4-unknown-linux-gnu, this patch causes two new failures on
libstdc++ testsuite
FAIL: 27_io/basic_ostream/inserters_arithmetic/char/7.cc execution test
FAIL: 27_io/basic_ostream/inserters_arithmetic/wchar_t/7.cc execution test
I can't find any differences between generated codes for those
test cases by compilers with/without your patch and the failures
go away if the tests are running with libstdc++ library built
with the unpatched compiler.
So it seems that something in libstdc++ library is miscompiled.
Weired and hard to see what is going on, ATM.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (2 preceding siblings ...)
2011-06-22 22:38 ` kkojima at gcc dot gnu.org
@ 2011-06-26 20:49 ` oleg.endo@t-online.de
2011-06-27 6:40 ` kkojima at gcc dot gnu.org
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: oleg.endo@t-online.de @ 2011-06-26 20:49 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
Oleg Endo <oleg.endo@t-online.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #24560|0 |1
is obsolete| |
--- Comment #4 from Oleg Endo <oleg.endo@t-online.de> 2011-06-26 20:48:19 UTC ---
Created attachment 24603
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24603
Proposed patch
Thanks for checking.
I missed to tell the expanders that the T bit is clobbered by insns such as
negdi2 and abssi2 / absdi2. The negdi2 expander that I have changed caused the
libstdc++ tests to fail when formatting a -1LL.
Another (reduced) example:
long long x (long long i, int j, long long k)
{
if (j & 5)
return -i;
return -k;
}
..ended up as (the "j & 5" got lost)...
mov.l @(4,r15),r1
clrt
mov.l @r15,r2
negc r1,r1
negc r2,r0
bt/s .L8
clrt
negc r5,r1
negc r4,r0
.L8:
rts
nop
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (3 preceding siblings ...)
2011-06-26 20:49 ` oleg.endo@t-online.de
@ 2011-06-27 6:40 ` kkojima at gcc dot gnu.org
2011-06-29 1:37 ` oleg.endo@t-online.de
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: kkojima at gcc dot gnu.org @ 2011-06-27 6:40 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
--- Comment #5 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-06-27 06:39:40 UTC ---
Argh, I also missed clobbers. Looks fine to me now, except
that insn_and_split "*negdi2" forgot to set constraints and
some minor coding style issues below.
The first comment should be started with a capital letter and
ended with a period. Also please follow GCC C coding style
even for C program segments in .md file. C lines in the patch
are started with a tab instead of 2 spaces. A long conditional
should be broken like as
(cond
? value0
: value1)
instead of
(cond ?
value0 :
value1)
Please use braces
"
{
int low_word = ...
...
emit_insn (...
DONE;
}")
instead of
"
int low_word = ...
...
emit_insn (...
DONE;
")
especially when new variables are used, though those braces
aren't required with the current gen* tools.
> + emit_insn (gen_negsi_cond (operands[0], operands[1], operands[1],
> + GEN_INT (1)));
The first line has an extra space after the last comma and
the indentation of the 2nd line doesn't match with GCC coding
standard. BTW, you could use const[01]_rtx for GEN_INT ([01]):
emit_insn (gen_negsi_cond (operands[0], operands[1], operands[1],
const1_rtx));
There are similar extra white space + broken indentation issues:
> +(define_insn_and_split "negsi_cond"
> + [(set (match_operand:SI 0 "arith_reg_dest" "=r,r")
> + (if_then_else:SI (eq:SI (reg:SI T_REG)
> + (match_operand:SI 3 "const_int_operand" "M,N"))
...
> + emit_label_after (skip_neg_label,
> + emit_insn (gen_negsi2 (operands[0], operands[1])));
...
Perhaps mail or editor problem?
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (4 preceding siblings ...)
2011-06-27 6:40 ` kkojima at gcc dot gnu.org
@ 2011-06-29 1:37 ` oleg.endo@t-online.de
2011-09-25 12:53 ` oleg.endo@t-online.de
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: oleg.endo@t-online.de @ 2011-06-29 1:37 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
Oleg Endo <oleg.endo@t-online.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #24603|0 |1
is obsolete| |
--- Comment #6 from Oleg Endo <oleg.endo@t-online.de> 2011-06-29 01:36:46 UTC ---
Created attachment 24625
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24625
Proposed patch
Added missing constraints to define_insn_and_split.
Fixed the formatting (hopefully ... there seems to be a problem with the tabs.
I'm using 4 spaces wide tabs)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (5 preceding siblings ...)
2011-06-29 1:37 ` oleg.endo@t-online.de
@ 2011-09-25 12:53 ` oleg.endo@t-online.de
2012-02-26 23:38 ` olegendo at gcc dot gnu.org
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: oleg.endo@t-online.de @ 2011-09-25 12:53 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
Oleg Endo <oleg.endo@t-online.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #24625|0 |1
is obsolete| |
--- Comment #7 from Oleg Endo <oleg.endo@t-online.de> 2011-09-25 12:48:24 UTC ---
Created attachment 25360
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25360
Proposed patch
The last version of the patch fails the test
gcc.c-torture/execute/arith-rand-ll.c for -m2a-single -mb and multiple
optimization levels with the following error:
internal compiler error: in change_address_1, at emit-rtl.c:1994
The attached version fixes some of the failures but still fails the test above
with -m2a-single -mb -O2. Other optimization levels work fine.
The problem is caused by the define_insn_and_split "*abssi2". It even fails if
the "*abssi2" splits into nothing but a simple register copy (movdi) or
comparison insn.
I'm now testing the patch without the DI abs parts and will submit it if passes
without new failures.
Cheers,
Oleg
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (6 preceding siblings ...)
2011-09-25 12:53 ` oleg.endo@t-online.de
@ 2012-02-26 23:38 ` olegendo at gcc dot gnu.org
2012-02-28 8:44 ` olegendo at gcc dot gnu.org
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-02-26 23:38 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
olegendo at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Last reconfirmed| |2012-02-26
CC| |olegendo at gcc dot gnu.org
AssignedTo|unassigned at gcc dot |olegendo at gcc dot gnu.org
|gnu.org |
Ever Confirmed|0 |1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (7 preceding siblings ...)
2012-02-26 23:38 ` olegendo at gcc dot gnu.org
@ 2012-02-28 8:44 ` olegendo at gcc dot gnu.org
2012-02-29 23:23 ` kkojima at gcc dot gnu.org
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-02-28 8:44 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
--- Comment #8 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-02-28 08:41:28 UTC ---
Created attachment 26768
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26768
Patch to add DImode abs
The attached patch adds DImode abs and did not introduce new failures when
tested against rev 184589.
I'm a bit confused, as this is exactly the same code as I used before to add
DImode abs, and before it was failing in some cases. Either there was another
bug that got fixed in the middle-end optimizers, or this is just coincidence.
I've seen that other expanders use force_reg to make sure that operands will be
in regs ... would that be the safer way of doing it?
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (8 preceding siblings ...)
2012-02-28 8:44 ` olegendo at gcc dot gnu.org
@ 2012-02-29 23:23 ` kkojima at gcc dot gnu.org
2012-03-14 23:10 ` olegendo at gcc dot gnu.org
2013-12-05 19:26 ` olegendo at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: kkojima at gcc dot gnu.org @ 2012-02-29 23:23 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
--- Comment #9 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-02-29 23:18:23 UTC ---
(In reply to comment #8)
Perhaps. Anyway looks fine to me except one minor failure
on sh64-elf:
xsh64-elf-combined/combined/libgcc/libgcc2.c: In function '__powisf2':
xsh64-elf-combined/combined/libgcc/libgcc2.c:1779:1: error: unrecognizable
insn:
(insn 11 10 12 3 (set (reg:DI 170)
(abs:DI (reg:DI 169)))
xsh64-elf-combined/combined/libgcc/libgcc2.c:1770 -1
(nil))
xsh64-elf-combined/combined/libgcc/libgcc2.c:1779:1: internal compiler error:
in extract_insn, at recog.c:2123
The failure went away if restricting new absdi2 expander to TARGET_SH1.
--- gcc/config/sh/sh.md~ 2012-02-29 10:52:16.000000000 +0900
+++ gcc/config/sh/sh.md 2012-02-29 11:07:42.000000000 +0900
@@ -4538,7 +4538,7 @@ label:
[(set (match_operand:DI 0 "arith_reg_dest" "")
(abs:DI (match_operand:DI 1 "arith_reg_operand" "")))
(clobber (reg:SI T_REG))]
- ""
+ "TARGET_SH1"
"")
(define_insn_and_split "*absdi2"
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (9 preceding siblings ...)
2012-02-29 23:23 ` kkojima at gcc dot gnu.org
@ 2012-03-14 23:10 ` olegendo at gcc dot gnu.org
2013-12-05 19:26 ` olegendo at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-03-14 23:10 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-03-14 22:58:05 UTC ---
Author: olegendo
Date: Wed Mar 14 22:57:58 2012
New Revision: 185397
URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185397
Log:
PR target/49468
* gcc.target/sh/pr49468-si.c: Make dg-skip-if not the first directive.
* gcc.target/sh/pr49468-di.c: Likewise.
Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/sh/pr49468-di.c
trunk/gcc/testsuite/gcc.target/sh/pr49468-si.c
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/49468] SH Target: inefficient integer abs code
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
` (10 preceding siblings ...)
2012-03-14 23:10 ` olegendo at gcc dot gnu.org
@ 2013-12-05 19:26 ` olegendo at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: olegendo at gcc dot gnu.org @ 2013-12-05 19:26 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468
Oleg Endo <olegendo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #11 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I think this can be closed.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-12-05 19:26 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-19 15:15 [Bug target/49468] New: SH Target: inefficient integer abs code oleg.endo@t-online.de
2011-06-19 15:30 ` [Bug target/49468] " oleg.endo@t-online.de
2011-06-19 15:49 ` oleg.endo@t-online.de
2011-06-22 22:38 ` kkojima at gcc dot gnu.org
2011-06-26 20:49 ` oleg.endo@t-online.de
2011-06-27 6:40 ` kkojima at gcc dot gnu.org
2011-06-29 1:37 ` oleg.endo@t-online.de
2011-09-25 12:53 ` oleg.endo@t-online.de
2012-02-26 23:38 ` olegendo at gcc dot gnu.org
2012-02-28 8:44 ` olegendo at gcc dot gnu.org
2012-02-29 23:23 ` kkojima at gcc dot gnu.org
2012-03-14 23:10 ` olegendo at gcc dot gnu.org
2013-12-05 19:26 ` olegendo 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).