On Fri, Nov 11, 2016 at 12:10:28PM +0100, Dominik Vogt wrote: > On Mon, Nov 07, 2016 at 09:29:26PM +0100, Bernd Schmidt wrote: > > On 10/31/2016 08:56 PM, Dominik Vogt wrote: > > > > >combine_simplify_rtx() tries to replace rtx expressions with just two > > >possible values with an experession that uses if_then_else: > > > > > > (if_then_else (condition) (value1) (value2)) > > > > > >If the original expression is e.g. > > > > > > (and (reg) (const_int 2)) > > > > I'm not convinced that if_then_else_cond is the right place to do > > this. That function is designed to answer the question of whether an > > rtx has exactly one of two values and under which condition; I feel > > it should continue to work this way. > > > > Maybe simplify_ternary_expression needs to be taught to deal with this case? > > But simplify_ternary_expression isn't called with the following > test program (only tried it on s390x): > > void bar(int, int); > int foo(int a, int *b) > { > if (a) > bar(0, *b & 2); > return *b; > } > > combine_simplify_rtx() is called with > > (sign_extend:DI (and:SI (reg:SI 61) (const_int 2))) > > In the switch it calls simplify_unary_operation(), which return > NULL. The next thing it does is call if_then_else_cond(), and > that calls itself with the sign_extend peeled off: > > (and:SI (reg:SI 61) (const_int 2)) > > takes the "BINARY_P (x)" path and returns false. The problem > exists only if the (and ...) is wrapped in ..._extend, i.e. the > ondition dealing with (and ...) directly can be removed from the > patch. > > So, all recursive calls to if_then_els_cond() return false, and > finally the condition in > > else if (HWI_COMPUTABLE_MODE_P (mode) > && pow2p_hwi (nz = nonzero_bits (x, mode)) > > is true. > > Thus, if if_then_else_cond should remain unchanged, the only place > to fix this would be after the call to if_then_else_cond() in > combine_simplify_rtx(). Actually, there already is some special > case handling to override the return code of if_then_else_cond(): > > cond = if_then_else_cond (x, &true_rtx, &false_rtx); > if (cond != 0 > /* If everything is a comparison, what we have is highly unlikely > to be simpler, so don't use it. */ > ---> && ! (COMPARISON_P (x) > && (COMPARISON_P (true_rtx) || COMPARISON_P (false_rtx)))) > { > rtx cop1 = const0_rtx; > enum rtx_code cond_code = simplify_comparison (NE, &cond, &cop1); > > ---> if (cond_code == NE && COMPARISON_P (cond)) > return x; > ... > > Should be easy to duplicate the test in the if-body, if that is > what you prefer: > > ... > if (HWI_COMPUTABLE_MODE_P (GET_MODE (x)) > && pow2p_hwi (nz = nonzero_bits (x, GET_MODE (x))) > && ! ((code == SIGN_EXTEND || code == ZERO_EXTEND) > && GET_CODE (XEXP (x, 0)) == AND > && CONST_INT_P (XEXP (XEXP (x, 0), 0)) > && UINTVAL (XEXP (XEXP (x, 0), 0)) == nz)) > return x; > > (untested)
Updated and tested version of the patch attached. The extra logic is now in combine_simplify_rtx. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
gcc/ChangeLog * combine.c (combine_simplify_rtx): Suppress replacement of "(and (reg) (const_int bit))" with "if_then_else".
>From 2ebe692928b4ebee3fa6dc02136980801a04b33d Mon Sep 17 00:00:00 2001 From: Dominik Vogt <v...@linux.vnet.ibm.com> Date: Mon, 31 Oct 2016 09:00:31 +0100 Subject: [PATCH] Do not simplify "(and (reg) (const bit)" to if_then_else. combine_simplify_rtx() tries to replace rtx expressions with just two possible values with an experession that uses if_then_else: (if_then_else (condition) (value1) (value2)) If the original expression is e.g. (and (reg) (const_int 2)) where the constant is the mask for a single bit, the replacement results in a more complex expression than before: (if_then_else (ne (zero_extract (reg) (1) (31))) (2) (0)) Similar replacements are done for (signextend (and ...)) (zeroextend (and ...)) Suppress the replacement this special case in if_then_else_cond(). --- gcc/combine.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/gcc/combine.c b/gcc/combine.c index b22a274..457fe8a 100644 --- a/gcc/combine.c +++ b/gcc/combine.c @@ -5575,10 +5575,23 @@ combine_simplify_rtx (rtx x, machine_mode op0_mode, int in_dest, { rtx cop1 = const0_rtx; enum rtx_code cond_code = simplify_comparison (NE, &cond, &cop1); + unsigned HOST_WIDE_INT nz; if (cond_code == NE && COMPARISON_P (cond)) return x; + /* If the operation is an AND wrapped in a SIGN_EXTEND or ZERO_EXTEND + with either operand being just a constant single bit value, do + nothing since IF_THEN_ELSE is likely to increase the expression's + complexity. */ + if (HWI_COMPUTABLE_MODE_P (mode) + && pow2p_hwi (nz = nonzero_bits (x, mode)) + && ! ((code == SIGN_EXTEND || code == ZERO_EXTEND) + && GET_CODE (XEXP (x, 0)) == AND + && CONST_INT_P (XEXP (XEXP (x, 0), 0)) + && UINTVAL (XEXP (XEXP (x, 0), 0)) == nz)) + return x; + /* Simplify the alternative arms; this may collapse the true and false arms to store-flag values. Be careful to use copy_rtx here since true_rtx or false_rtx might share RTL with x as a -- 2.3.0