Fix if-conversion bug reversing conditions

Fix if-conversion bug reversing conditions

[Joseph S. Myers]

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

Re: Fix if-conversion bug reversing conditions

[Richard Henderson]

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~

Re: Fix if-conversion bug reversing conditions

[Richard Guenther]

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.

Re: Fix if-conversion bug reversing conditions

[Joseph S. Myers]

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