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)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Reply via email to