Re: Fix COND_EXPR foldings that are not happening

[Roger Sayle <roger at eyesopen dot com>]

On Fri, 18 Jun 2004, Paolo Bonzini wrote:
> 2004-06-11  Paolo Bonzini  <bonzini@gnu.org>
>
>       * fold-const.c (fold_cond_expr_with_comparison):
>       New function, extracted from fold.
>       (fold): Extract code to fold A op B ? A : C, use
>       it to fold A op B ? C : A.  Really optimize
>       A & N ? N : 0 where N is a power of two.  Avoid
>       relying on canonicalization and recursion for
>       foldings of COND_EXPR to happen.

This looks much better, thanks.  This is OK for mainline with the two
minor tweaks below.


+ /* Subroutine of fold, looking inside expressions of the form
+    A op B ? A : C.  */
+ static tree
+ fold_cond_expr_with_comparison (tree type, tree arg0, tree arg2)

Could you expand upon the documentation of this new function a bit?
It might not be immediately clear to someone reading it why it's
passed arg0 and arg2, but not arg1, and whether its allowed to return
a NULL_TREE or not.  I believe the policy is that we should try to
describe each of the arguments and the return value in the comment
above each function.


!       /* Look for expressions of the form A & 2 ? 2 : 0.  The result of this
!        operation is simply A & 2.  */

!       if (integer_zerop (TREE_OPERAND (t, 2))
!         && TREE_CODE (arg0) == NE_EXPR
!         && integer_zerop (TREE_OPERAND (arg0, 1))
!         && integer_pow2p (arg1)
!         && TREE_CODE (TREE_OPERAND (arg0, 0)) == BIT_AND_EXPR
!         && operand_equal_p (TREE_OPERAND (TREE_OPERAND (arg0, 0), 1),
!                             arg1, OEP_ONLY_CONST))
!       return pedantic_non_lvalue (fold_convert (type,
!                                                 TREE_OPERAND (arg0, 0)));

It looks like you're still removing this optimization, rather than just
adding /* A < 0 ? <sign bit of A> : 0 is simply (A & <sign bit of A>).  */
and /* (A >> N) & 1 ? (1 << N) : 0 is simply A & (1 << N).  */

Admittedly, this shouldn't trigger if the COND_EXPR's operands have
already been folded, but I'd prefer that we retain this transformation
in "fold" in case, we change the set of transformations that get applied
at some point in the future.


I suspect that its just an oversight that you've again forgotten to
mention the target triple that you've benchmarked and regression tested
on :>  However please bootstrap and regression test again with the above
changes confirming there are no new regressions before committing to CVS.

Thanks again for making these changes,

Roger
--