public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
@ 2012-10-17 23:49 kkojima at gcc dot gnu.org
2012-10-20 6:33 ` [Bug target/54963] [4.8 Regression] " kkojima at gcc dot gnu.org
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: kkojima at gcc dot gnu.org @ 2012-10-17 23:49 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
Bug #: 54963
Summary: Wrong code generated for
libgfortran/generated/eoshift3_8.c on SH
Classification: Unclassified
Product: gcc
Version: 4.8.0
Status: UNCONFIRMED
Keywords: wrong-code
Severity: normal
Priority: P3
Component: middle-end
AssignedTo: unassigned@gcc.gnu.org
ReportedBy: kkojima@gcc.gnu.org
Target: sh*-*-*
Created attachment 28469
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28469
A test case
Several fortran tests using some eoshift functions fail on
sh4-unknown-linux-gnu for a while. The above test case is
a reduced one which fails sh-elf with -O2 -ml -m4. The right
result would be "adhbeh..." but it wrongly returns "ticzzz...".
It seems that the compiler allocates a stack slot at r15+60
for "len" variable first for the line 126:
len = ((array)->dim[dim]._ubound + 1 - (array)->dim[dim].lower_bound);
then computes len - abs (sh) for the line 184;
for (n = 0; n < len - delta; n++)
using "len" variable at r15+68. The error went away with
-fno-ira-share-spill-slots.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
@ 2012-10-20 6:33 ` kkojima at gcc dot gnu.org
2012-10-20 9:29 ` olegendo at gcc dot gnu.org
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: kkojima at gcc dot gnu.org @ 2012-10-20 6:33 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
Kazumoto Kojima <kkojima at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |olegendo at gcc dot gnu.org
Component|middle-end |target
Summary|Wrong code generated for |[4.8 Regression] Wrong code
|libgfortran/generated/eoshi |generated for
|ft3_8.c on SH |libgfortran/generated/eoshi
| |ft3_8.c on SH
--- Comment #1 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-10-20 06:33:06 UTC ---
It looks that the problematic code run through a wrong flow
and the above was misleading. It seems that the change
r184829 | olegendo | 2012-03-03 06:21:13 +0900 (Sat, 03 Mar 2012) | 8 lines
PR target/49486
* config/sh/sh.md (negdi2): Add TARGET_SH1 condition.
(absdi2): New expander.
(*absdi2, *negabsdi2, negdi_cond): New insns and splits.
* gcc.target/sh/pr49468-si.c: Skip unsupported test for SH64.
* gcc.target/sh/pr49468-di.c: New.
causes this. After reload, there is an insn for abs(sh):
(insn 432 431 1393 39 (set (reg:DI 2 r2)
(abs:DI (reg:DI 1 r1))) eoshift.c:167 189 {*absdi2}
(nil))
which is splitted into
(insn 1607 431 1609 39 (set (reg:SI 147 t)
(ge:SI (reg:SI 2 r2 [+4 ])
(const_int 0 [0]))) eoshift.c:167 13 {cmpgesi_t}
(nil))
(insn 1609 1607 1610 39 (set (reg:SI 2 r2)
(reg:SI 1 r1)) eoshift.c:167 234 {movsi_ie}
(nil))
(insn 1610 1609 1611 39 (set (reg:SI 3 r3 [+4 ])
(reg:SI 2 r2 [+4 ])) eoshift.c:167 234 {movsi_ie}
(nil))
(jump_insn 1611 1610 1626 39 (set (pc)
(if_then_else (ne (reg:SI 147 t)
(const_int 0 [0]))
...
The insn 1609 changes r2 before its original value is used.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
2012-10-20 6:33 ` [Bug target/54963] [4.8 Regression] " kkojima at gcc dot gnu.org
@ 2012-10-20 9:29 ` olegendo at gcc dot gnu.org
2012-10-28 23:01 ` olegendo at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-10-20 9:29 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
Oleg Endo <olegendo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Last reconfirmed| |2012-10-20
AssignedTo|unassigned at gcc dot |olegendo at gcc dot gnu.org
|gnu.org |
Ever Confirmed|0 |1
--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-20 09:29:10 UTC ---
Thanks for tracking this one down. The buggy lines are in the negdi_cond
pattern:
emit_insn (gen_movsi (low_dst, low_src));
emit_insn (gen_movsi (high_dst, high_src));
I'll have a look at it.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
2012-10-20 6:33 ` [Bug target/54963] [4.8 Regression] " kkojima at gcc dot gnu.org
2012-10-20 9:29 ` olegendo at gcc dot gnu.org
@ 2012-10-28 23:01 ` olegendo at gcc dot gnu.org
2012-10-29 0:59 ` kkojima at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-10-28 23:01 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-28 23:01:24 UTC ---
Created attachment 28551
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28551
Proposed patch
This patch fixes the problem, by using 'emit_move_insn' instead of manually
doing the DImode reg copy.
I've seized the moment and refactored the abs patterns -- the T_REG clobber
handling was a bit confusing and using mode iterators saves a few lines.
Kaz, could you please have a look at this one?
Only briefly tested with make-gcc and compiling CSiBE. There are no code size
changes in the CSiBE set, except for one:
jikespg-1.3
src/prntstat 3576 -> 3568 -8 / -0.223714 %
I guess it's the T_REG clobber thing that has a positive impact on register
allocation in this case.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
` (2 preceding siblings ...)
2012-10-28 23:01 ` olegendo at gcc dot gnu.org
@ 2012-10-29 0:59 ` kkojima at gcc dot gnu.org
2012-10-29 11:13 ` olegendo at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: kkojima at gcc dot gnu.org @ 2012-10-29 0:59 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
--- Comment #4 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-10-29 00:59:37 UTC ---
(In reply to comment #3)
> Created attachment 28551 [details]
> Proposed patch
>
> This patch fixes the problem, by using 'emit_move_insn' instead of manually
> doing the DImode reg copy.
Does the pattern in negdi_cond
emit_insn (gen_negc (low_dst, low_src));
emit_label_after (skip_neg_label, emit_insn (gen_negc (high_dst, high_src)));
work in the problematic situation? Perhaps I've missed something.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
` (3 preceding siblings ...)
2012-10-29 0:59 ` kkojima at gcc dot gnu.org
@ 2012-10-29 11:13 ` olegendo at gcc dot gnu.org
2012-10-30 1:04 ` olegendo at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-10-29 11:13 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
--- Comment #5 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-29 11:13:19 UTC ---
(In reply to comment #4)
> (In reply to comment #3)
> > Created attachment 28551 [details]
> > Proposed patch
> >
> > This patch fixes the problem, by using 'emit_move_insn' instead of manually
> > doing the DImode reg copy.
>
> Does the pattern in negdi_cond
>
> emit_insn (gen_negc (low_dst, low_src));
> emit_label_after (skip_neg_label, emit_insn (gen_negc (high_dst, high_src)));
>
> work in the problematic situation? Perhaps I've missed something.
Ugh, you're right. The negdi will go wrong if input and output regs overlap.
I guess making the output operand a "=&r" for DImode should fix the issue (as
it's done in the adddi3 or subdi3 patterns). Moreover, I think it should be OK
to split up the absdi pattern before reload, except for negdi_cond. I'll try
that out.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
` (4 preceding siblings ...)
2012-10-29 11:13 ` olegendo at gcc dot gnu.org
@ 2012-10-30 1:04 ` olegendo at gcc dot gnu.org
2012-10-30 2:46 ` kkojima at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-10-30 1:04 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
Oleg Endo <olegendo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #28551|0 |1
is obsolete| |
--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-30 01:04:10 UTC ---
Created attachment 28567
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28567
Proposed patch
I'm now testing this patch. It fixes the overlapping reg negdi problem.
There's a code size increase in CSiBE:
linux-2.4.23-pre3-testplatform
lib/vsprintf 4776 -> 4820 +44 / +0.921273 %
I haven't checked the details of this case, but I guess this is the price of
correct code ;)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
` (5 preceding siblings ...)
2012-10-30 1:04 ` olegendo at gcc dot gnu.org
@ 2012-10-30 2:46 ` kkojima at gcc dot gnu.org
2012-10-30 9:23 ` olegendo at gcc dot gnu.org
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: kkojima at gcc dot gnu.org @ 2012-10-30 2:46 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
--- Comment #7 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-10-30 02:46:09 UTC ---
(In reply to comment #6)
Agreed, patch is pre-approved.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
` (6 preceding siblings ...)
2012-10-30 2:46 ` kkojima at gcc dot gnu.org
@ 2012-10-30 9:23 ` olegendo at gcc dot gnu.org
2012-11-01 0:01 ` olegendo at gcc dot gnu.org
2012-11-01 0:13 ` kkojima at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-10-30 9:23 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
--- Comment #8 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-30 09:22:31 UTC ---
Author: olegendo
Date: Tue Oct 30 09:22:14 2012
New Revision: 192983
URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192983
Log:
PR target/54963
* config/sh/iterators.md (SIDI): New mode iterator.
* config/sh/sh.md (negdi2): Use parallel around operation and T_REG
clobber in expander.
(*negdi2): Mark output operand as early clobbered. Add T_REG clobber.
Split after reload. Simplify split code.
(abssi2, absdi2): Fold expanders into abs<mode>2.
(*abssi2, *absdi2): Fold into *abs<mode>2 insn_and_split. Split insns
before reload.
(*negabssi2, *negabsdi2): Fold into *negabs<mode>2. Add T_REG clobber.
Split insns before reload.
(negsi_cond): Reformat. Use emit_move_insn instead of
gen_movesi.
(negdi_cond): Reformat. Use emit_move_insn instead of a pair
of gen_movsi. Split insn before reload.
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/sh/iterators.md
trunk/gcc/config/sh/sh.md
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
` (7 preceding siblings ...)
2012-10-30 9:23 ` olegendo at gcc dot gnu.org
@ 2012-11-01 0:01 ` olegendo at gcc dot gnu.org
2012-11-01 0:13 ` kkojima at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: olegendo at gcc dot gnu.org @ 2012-11-01 0:01 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-11-01 00:01:13 UTC ---
Kaz, can we close this PR?
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/54963] [4.8 Regression] Wrong code generated for libgfortran/generated/eoshift3_8.c on SH
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
` (8 preceding siblings ...)
2012-11-01 0:01 ` olegendo at gcc dot gnu.org
@ 2012-11-01 0:13 ` kkojima at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: kkojima at gcc dot gnu.org @ 2012-11-01 0:13 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54963
Kazumoto Kojima <kkojima at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution| |FIXED
--- Comment #10 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-11-01 00:13:13 UTC ---
Yes. Closed as FIXED.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-11-01 0:13 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-17 23:49 [Bug middle-end/54963] New: Wrong code generated for libgfortran/generated/eoshift3_8.c on SH kkojima at gcc dot gnu.org
2012-10-20 6:33 ` [Bug target/54963] [4.8 Regression] " kkojima at gcc dot gnu.org
2012-10-20 9:29 ` olegendo at gcc dot gnu.org
2012-10-28 23:01 ` olegendo at gcc dot gnu.org
2012-10-29 0:59 ` kkojima at gcc dot gnu.org
2012-10-29 11:13 ` olegendo at gcc dot gnu.org
2012-10-30 1:04 ` olegendo at gcc dot gnu.org
2012-10-30 2:46 ` kkojima at gcc dot gnu.org
2012-10-30 9:23 ` olegendo at gcc dot gnu.org
2012-11-01 0:01 ` olegendo at gcc dot gnu.org
2012-11-01 0:13 ` kkojima 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).