From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12738 invoked by alias); 29 Sep 2008 17:12:51 -0000 Received: (qmail 12728 invoked by uid 22791); 29 Sep 2008 17:12:50 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 29 Sep 2008 17:11:58 +0000 Received: (qmail 2145 invoked from network); 29 Sep 2008 17:11:56 -0000 Received: from unknown (HELO digraph.polyomino.org.uk) (joseph@127.0.0.2) by mail.codesourcery.com with ESMTPA; 29 Sep 2008 17:11:56 -0000 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.68) (envelope-from ) id 1KkMHr-0004NB-Dv for gcc-patches@gcc.gnu.org; Mon, 29 Sep 2008 17:11:55 +0000 Date: Mon, 29 Sep 2008 17:49:00 -0000 From: "Joseph S. Myers" To: gcc-patches@gcc.gnu.org Subject: Fix if-conversion bug reversing conditions Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2008-09/txt/msg01944.txt.bz2 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 * 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