* Fix if-conversion bug reversing conditions
@ 2008-09-29 17:49 Joseph S. Myers
2008-09-29 19:07 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: Joseph S. Myers @ 2008-09-29 17:49 UTC (permalink / raw)
To: gcc-patches
This patch fixes an if-conversion problem that manifested itself in a
4.3-based toolchain as execution failures on powerpc-linux-gnu
-msoft-float:
FAIL: gcc.c-torture/execute/stdarg-3.c execution, -O3 -fomit-frame-pointer -funroll-loops
FAIL: gcc.c-torture/execute/stdarg-3.c execution, -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions
I don't have a testcase manifesting the bug on trunk but believe it is
still present there.
A loop is unrolled, and an initial test for whether it has an even or
an odd number of iterations gets wrongly inverted by the post-combine
if-conversion pass (ce2). After combine the relevant insns (for a
reduced version of stdarg-3.c with the 4.3-based toolchain) look like:
(insn 144 113 145 3 (parallel [
(set (reg:CC 166)
(compare:CC (zero_extract:SI (reg/v:SI 120 [ j ])
(const_int 1 [0x1])
(const_int 31 [0x1f]))
(const_int 0 [0x0])))
(clobber (scratch:SI))
]) 159 {*extzvsi_internal1} (nil))
(jump_insn 145 144 70 3 (set (pc)
(if_then_else (ne (reg:CC 166)
(const_int 0 [0x0]))
(label_ref:SI 178)
(pc))) 560 {*rs6000.md:13937} (expr_list:REG_DEAD (reg:CC 166)
(expr_list:REG_BR_PROB (const_int 5000 [0x1388])
(nil))))
;; End of basic block 3 -> ( 7 4)
;; Succ edge 7 [50.0%]
;; Succ edge 4 [50.0%] (fallthru)
...
;; Start of basic block ( 3) -> 7
;; Pred edge 3 [50.0%]
(code_label 178 81 137 7 11 "" [1 uses])
(note 137 178 139 7 [bb 7] NOTE_INSN_BASIC_BLOCK)
(insn 139 137 181 7 stdarg-3.c:13 (set (reg/v:SI 120 [ j ])
(plus:SI (reg/v:SI 120 [ j ])
(const_int -1 [0xffffffffffffffff]))) 79 {*addsi3_internal1} (nil))
(jump_insn 181 139 182 7 (set (pc)
(label_ref 70)) -1 (nil))
;; End of basic block 7 -> ( 4)
find_if_header duly enters noce_find_if_block with then_edge being the
EDGE_FALLTHRU edge (to block 4) and ELSE_EDGE being the edge to block
7. This reaches the IF-ELSE-JOIN case and sets then_else_reversed, so
then_bb, insn_a as set in noce_process_if_block, is now block 7.
noce_get_condition duly inverts the jump condition. This is correct
so far; if_info->cond is meant to be the condition for going to
if_info->insn_b (though the comments never say so explicitly, a lot of
code only makes sense that way).
The code in question - subtracting 1 from a REG in the insn_a case
(i.e. if if_info->cond is false) - is handled by noce_try_addcc. This
reaches the
/* If that fails, construct conditional increment or decrement using
setcc. */
case and so calls noce_emit_store_flag with REVERSEP set to 1. Thus
noce_emit_store_flag knows to store the reverse of if_info->cond,
which would be the right value to subtract. However, it then runs
into the case
/* If earliest == jump, or when the condition is complex, try to
build the store_flag insn directly. */
where it uses the condition from the original jump, ignoring the
possibility of this condition having been reversed in order to get the
correct insn_b condition.
This patch makes noce_emit_store_flag check the same conditions as
noce_get_condition for whether to reverse the condition, when it goes
back to the original jump.
Bootstrapped with no regressions on i686-pc-linux-gnu. Tested with no
regressions with cross to powerpc-linux-gnu -msoft-float. OK to
commit?
2008-09-29 Joseph Myers <joseph@codesourcery.com>
* ifcvt.c (noce_emit_store_flag): If using condition from original
jump, reverse it if if_info->cond was reversed.
Index: ifcvt.c
===================================================================
--- ifcvt.c (revision 140752)
+++ ifcvt.c (working copy)
@@ -666,7 +666,15 @@
build the store_flag insn directly. */
if (cond_complex)
- cond = XEXP (SET_SRC (pc_set (if_info->jump)), 0);
+ {
+ rtx set = pc_set (if_info->jump);
+ cond = XEXP (SET_SRC (set), 0);
+ if (GET_CODE (XEXP (SET_SRC (set), 2)) == LABEL_REF
+ && XEXP (XEXP (SET_SRC (set), 2), 0) == JUMP_LABEL (if_info->jump))
+ reversep = !reversep;
+ if (if_info->then_else_reversed)
+ reversep = !reversep;
+ }
if (reversep)
code = reversed_comparison_code (cond, if_info->jump);
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Fix if-conversion bug reversing conditions
2008-09-29 17:49 Fix if-conversion bug reversing conditions Joseph S. Myers
@ 2008-09-29 19:07 ` Richard Henderson
2008-09-30 13:05 ` Richard Guenther
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2008-09-29 19:07 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: gcc-patches
Joseph S. Myers wrote:
> * ifcvt.c (noce_emit_store_flag): If using condition from original
> jump, reverse it if if_info->cond was reversed.
Ok.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Fix if-conversion bug reversing conditions
2008-09-29 19:07 ` Richard Henderson
@ 2008-09-30 13:05 ` Richard Guenther
2008-09-30 20:20 ` Joseph S. Myers
0 siblings, 1 reply; 4+ messages in thread
From: Richard Guenther @ 2008-09-30 13:05 UTC (permalink / raw)
To: Richard Henderson; +Cc: Joseph S. Myers, gcc-patches
On Mon, Sep 29, 2008 at 7:44 PM, Richard Henderson <rth@redhat.com> wrote:
> Joseph S. Myers wrote:
>>
>> * ifcvt.c (noce_emit_store_flag): If using condition from original
>> jump, reverse it if if_info->cond was reversed.
>
> Ok.
IMO as a wrong-code bug this also qualifies it for the branches.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Fix if-conversion bug reversing conditions
2008-09-30 13:05 ` Richard Guenther
@ 2008-09-30 20:20 ` Joseph S. Myers
0 siblings, 0 replies; 4+ messages in thread
From: Joseph S. Myers @ 2008-09-30 20:20 UTC (permalink / raw)
To: Richard Guenther; +Cc: Richard Henderson, gcc-patches
On Tue, 30 Sep 2008, Richard Guenther wrote:
> On Mon, Sep 29, 2008 at 7:44 PM, Richard Henderson <rth@redhat.com> wrote:
> > Joseph S. Myers wrote:
> >>
> >> * ifcvt.c (noce_emit_store_flag): If using condition from original
> >> jump, reverse it if if_info->cond was reversed.
> >
> > Ok.
>
> IMO as a wrong-code bug this also qualifies it for the branches.
Now bootstrapped with no regressions on i686-pc-linux-gnu on 4.3 branch
and committed there.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-09-30 19:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-29 17:49 Fix if-conversion bug reversing conditions Joseph S. Myers
2008-09-29 19:07 ` Richard Henderson
2008-09-30 13:05 ` Richard Guenther
2008-09-30 20:20 ` Joseph S. Myers
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).