Ping.
On Mon, Nov 21, 2016 at 01:36:47PM +0100, Dominik Vogt wrote:
> 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.
> 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
>
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
