* [Bug tree-optimization/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
@ 2014-11-20 9:55 ` rguenth at gcc dot gnu.org
2014-11-20 9:57 ` rguenth at gcc dot gnu.org
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-11-20 9:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Last reconfirmed| |2014-11-20
Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org
Target Milestone|--- |5.0
Ever confirmed|0 |1
--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think you were mostly lucky -- 4.9 expands from
<bb 2>:
_3 = a_2(D) == 0;
_13 = ~_3;
_7 = (int) _13;
MEM[(int *)d_5(D) + 8B] = _7;
if (_3 != 0)
which it reaches in a very weird way through several optimization passes
while trunk now has
<bb 2>:
_7 = a_2(D) != 0;
_8 = (int) _7;
MEM[(int *)d_5(D) + 8B] = _8;
if (a_2(D) == 0)
which has un-CSEd the a_2 == 0 comparison due to a bug I introduced.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug tree-optimization/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
2014-11-20 9:55 ` [Bug tree-optimization/63986] " rguenth at gcc dot gnu.org
@ 2014-11-20 9:57 ` rguenth at gcc dot gnu.org
2014-11-20 11:42 ` ebotcazou at gcc dot gnu.org
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-11-20 9:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |missed-optimization
--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Btw, x86 code generation is the same:
test_0:
.LFB0:
.cfi_startproc
xorl %eax, %eax
testl %edi, %edi
setne %al
movl %eax, 8(%rcx)
movl %edx, %eax
cmove %esi, %eax
ret
so somehow it manages to fix things up again (maybe through ifcvt).
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug tree-optimization/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
2014-11-20 9:55 ` [Bug tree-optimization/63986] " rguenth at gcc dot gnu.org
2014-11-20 9:57 ` rguenth at gcc dot gnu.org
@ 2014-11-20 11:42 ` ebotcazou at gcc dot gnu.org
2014-11-20 13:13 ` ppalka at gcc dot gnu.org
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2014-11-20 11:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |ebotcazou at gcc dot gnu.org
--- Comment #4 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I suppose RTL CSE cannot CSE flag register sets...?
Yes, it can, if you define
-- Target Hook: bool TARGET_FIXED_CONDITION_CODE_REGS (unsigned int
*P1, unsigned int *P2)
On targets which do not use `(cc0)', and which use a hard register
rather than a pseudo-register to hold condition codes, the regular
CSE passes are often not able to identify cases in which the hard
register is set to a common value. Use this hook to enable a
small pass which optimizes such cases. This hook should return
true to enable this pass, and it should set the integers to which
its arguments point to the hard register numbers used for
condition codes. When there is only one such register, as is true
on most systems, the integer pointed to by P2 should be set to
`INVALID_REGNUM'.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug tree-optimization/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (2 preceding siblings ...)
2014-11-20 11:42 ` ebotcazou at gcc dot gnu.org
@ 2014-11-20 13:13 ` ppalka at gcc dot gnu.org
2014-11-20 13:29 ` rguenther at suse dot de
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: ppalka at gcc dot gnu.org @ 2014-11-20 13:13 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
--- Comment #5 from ppalka at gcc dot gnu.org ---
(In reply to Richard Biener from comment #3)
> Ok, now already existing forwprop code gets fed with
>
> <bb 2>:
> _3 = a_2(D) == 0;
> x_4 = (char) _3;
> _7 = ~_3;
> _8 = (int) _7;
> MEM[(int *)d_5(D) + 8B] = _8;
> if (x_4 != 0)
>
> where we now in the first forwprop pass identified the opportunity
> to use ~_3 instead of x_4 == 0 thus x_4 is now no longer multi-use.
> This makes us optimize if (x_4 != 0) to if (_3 != 0) which we
> re-optimize in fold_gimple_cond now to '_3' and then of course
> if (_3 != 0) (err, and we return "changed"....) which means we
> now propagate _again_ via forward_propagate_into_gimple_cond
> which now specifically allows aggressive forwarding of compares,
> bypassing single-use restrictions. See
>
> 2014-11-16 Patrick Palka <ppalka@gcc.gnu.org>
>
> PR middle-end/63790
> * tree-ssa-forwprop.c (forward_propagate_into_comparison_1):
> Always combine comparisons or conversions from booleans.
>
> thus me fixing my "mistake" does not help anymore.
I worried that such an issue might pop up...
If you think my patch should be reverted, note that there is a more benign
version of the patch here
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00963.html (which still fixes the
test case I was originally interested in fixing).
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug tree-optimization/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (3 preceding siblings ...)
2014-11-20 13:13 ` ppalka at gcc dot gnu.org
@ 2014-11-20 13:29 ` rguenther at suse dot de
2014-11-20 18:15 ` olegendo at gcc dot gnu.org
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenther at suse dot de @ 2014-11-20 13:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 20 Nov 2014, ppalka at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
>
> --- Comment #5 from ppalka at gcc dot gnu.org ---
> (In reply to Richard Biener from comment #3)
> > Ok, now already existing forwprop code gets fed with
> >
> > <bb 2>:
> > _3 = a_2(D) == 0;
> > x_4 = (char) _3;
> > _7 = ~_3;
> > _8 = (int) _7;
> > MEM[(int *)d_5(D) + 8B] = _8;
> > if (x_4 != 0)
> >
> > where we now in the first forwprop pass identified the opportunity
> > to use ~_3 instead of x_4 == 0 thus x_4 is now no longer multi-use.
> > This makes us optimize if (x_4 != 0) to if (_3 != 0) which we
> > re-optimize in fold_gimple_cond now to '_3' and then of course
> > if (_3 != 0) (err, and we return "changed"....) which means we
> > now propagate _again_ via forward_propagate_into_gimple_cond
> > which now specifically allows aggressive forwarding of compares,
> > bypassing single-use restrictions. See
> >
> > 2014-11-16 Patrick Palka <ppalka@gcc.gnu.org>
> >
> > PR middle-end/63790
> > * tree-ssa-forwprop.c (forward_propagate_into_comparison_1):
> > Always combine comparisons or conversions from booleans.
> >
> > thus me fixing my "mistake" does not help anymore.
>
> I worried that such an issue might pop up...
>
> If you think my patch should be reverted, note that there is a more benign
> version of the patch here
> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00963.html (which still fixes the
> test case I was originally interested in fixing).
No, I think that conceptually aggressive forwarding into if conditions
is wanted. I believe it's not impossible to fix the very special
cases that might pop up - usually it will help code generation.
Richard.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug tree-optimization/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (4 preceding siblings ...)
2014-11-20 13:29 ` rguenther at suse dot de
@ 2014-11-20 18:15 ` olegendo at gcc dot gnu.org
2014-11-21 8:29 ` [Bug target/63986] " rguenth at gcc dot gnu.org
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: olegendo at gcc dot gnu.org @ 2014-11-20 18:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> I think you were mostly lucky
And exactly that's why I'd better not rely on combine dreaming up the parallel
pattern if I want to control the instruction (sequence) selection in a more
reliable way.
I didn't mean to file this PR as tree-optimization, but rather as a target
issue.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (5 preceding siblings ...)
2014-11-20 18:15 ` olegendo at gcc dot gnu.org
@ 2014-11-21 8:29 ` rguenth at gcc dot gnu.org
2014-11-22 6:40 ` olegendo at gcc dot gnu.org
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-11-21 8:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Component|tree-optimization |target
--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Right. Let's put it back to target land then. I think what the tree level
does
is reasonable.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (6 preceding siblings ...)
2014-11-21 8:29 ` [Bug target/63986] " rguenth at gcc dot gnu.org
@ 2014-11-22 6:40 ` olegendo at gcc dot gnu.org
2014-11-22 15:07 ` olegendo at gcc dot gnu.org
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: olegendo at gcc dot gnu.org @ 2014-11-22 6:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Created attachment 34073
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34073&action=edit
A possible patch
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (7 preceding siblings ...)
2014-11-22 6:40 ` olegendo at gcc dot gnu.org
@ 2014-11-22 15:07 ` olegendo at gcc dot gnu.org
2014-11-25 6:46 ` olegendo at gcc dot gnu.org
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: olegendo at gcc dot gnu.org @ 2014-11-22 15:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Sat Nov 22 15:06:34 2014
New Revision: 217968
URL: https://gcc.gnu.org/viewcvs?rev=217968&root=gcc&view=rev
Log:
gcc/
PR target/63986
PR target/51244
* config/sh/sh.c (sh_is_logical_t_store_expr,
sh_try_omit_signzero_extend): Use rtx_insn* for insn argument.
(sh_split_movrt_negc_to_movt_xor): New function.
(sh_find_set_of_reg): Move to ...
* config/sh/sh-protos.h (sh_find_set_of_reg): ... here and convert
to template function.
(set_of_reg): Use rtx_insn* for insn member.
(sh_is_logical_t_store_expr, sh_try_omit_signzero_extend): Use
rtx_insn* for insn argument.
* config/sh/sh.md (movrt_negc, *movrt_negc): Split into movt-xor
sequence using new sh_split_movrt_negc_to_movt_xor function.
(movrt_xor): Allow also for SH2A.
(*movt_movrt): Delete insns and splits.
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/sh/sh-protos.h
trunk/gcc/config/sh/sh.c
trunk/gcc/config/sh/sh.md
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (8 preceding siblings ...)
2014-11-22 15:07 ` olegendo at gcc dot gnu.org
@ 2014-11-25 6:46 ` olegendo at gcc dot gnu.org
2014-12-01 6:50 ` olegendo at gcc dot gnu.org
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: olegendo at gcc dot gnu.org @ 2014-11-25 6:46 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
--- Comment #11 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #10)
> Author: olegendo
> Date: Sat Nov 22 15:06:34 2014
> New Revision: 217968
>
> URL: https://gcc.gnu.org/viewcvs?rev=217968&root=gcc&view=rev
Hm, I think the patch is missing some additional checks on the operands of the
removed comparison insn, which might result in wrong code. Please leave this
PR open for now.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (9 preceding siblings ...)
2014-11-25 6:46 ` olegendo at gcc dot gnu.org
@ 2014-12-01 6:50 ` olegendo at gcc dot gnu.org
2014-12-10 15:32 ` rguenth at gcc dot gnu.org
2014-12-10 20:36 ` olegendo at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: olegendo at gcc dot gnu.org @ 2014-12-01 6:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
--- Comment #12 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Mon Dec 1 06:50:06 2014
New Revision: 218200
URL: https://gcc.gnu.org/viewcvs?rev=218200&root=gcc&view=rev
Log:
gcc/
PR target/63986
PR target/51244
* config/sh/sh.c (sh_unspec_insn_p,
sh_insn_operands_modified_between_p): New functions.
(sh_split_movrt_negc_to_movt_xor): Do not delete insn if its operands
are modified or if it has side effects, may trap or is volatile.
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/sh/sh.c
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (10 preceding siblings ...)
2014-12-01 6:50 ` olegendo at gcc dot gnu.org
@ 2014-12-10 15:32 ` rguenth at gcc dot gnu.org
2014-12-10 20:36 ` olegendo at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-12-10 15:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/63986] [5 Regression][SH] gcc.target/sh/pr51244-15.c failures
2014-11-20 3:51 [Bug tree-optimization/63986] New: [5 Regression][SH] gcc.target/sh/pr51244-15.c failures olegendo at gcc dot gnu.org
` (11 preceding siblings ...)
2014-12-10 15:32 ` rguenth at gcc dot gnu.org
@ 2014-12-10 20:36 ` olegendo at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: olegendo at gcc dot gnu.org @ 2014-12-10 20:36 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63986
Oleg Endo <olegendo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #14 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #13)
> Fixed?
Let's assume so.
^ permalink raw reply [flat|nested] 14+ messages in thread