Re: Move ABS detection from fold-const.c to match.pd
On Sun, Jun 28, 2015 at 8:34 PM, Marc Glisse marc.gli...@inria.fr wrote: (this message looks like it was lost in my draft folder...) On Tue, 26 May 2015, Richard Biener wrote: +(match zerop integer_zerop) +(match zerop real_zerop) Would it also include fixed_zerop? Probably, yes. The main issue is that I know next to nothing about fixed-point types, so I am always unsure how to handle them (when I don't forget them completely). For instance, in the recently added -A CMP -B, we could probably replace (if (FLOAT_TYPE_P (TREE_TYPE (@0)) || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0)) TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0 with (if (FLOAT_TYPE_P (TREE_TYPE (@0)) || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))) Not sure if TYPE_OVERFLOW_UNDEFINED says sth sensible for fixed-point types given that there is no overflow for them but they saturate. As far as I see the check would even ICE without guarding it with ANY_INTEGRAL_TYPE_P. So it would be || NON_SAT_FIXED_POINT_TYPE_P (TREE_TYPE (@0)) where I am not sure whether overflow is undefined for non-saturating fixed-point types ... Note that with inlining implemented it would duplicate the pattern for each match variant thus in this case adding a tree.[ch] function zerop () might be better. Ah... I actually thought we might end up moving things like integer_zerop from tree.c to match.pd, especially since predicates are not declared 'static'... Ok, reverse gear. Yeah, I don't think match.pd is a good fit for them. Note that inlining does not seem necessary to implement more advanced predicates like negated_value_for_comparison in the parent message. Sure not necessary but one point of match-and-simplify was that the pattern matching is fast because it uses a decision tree. Once you introduce predicates that are in functions with their own decision tree you get back to testing all of them. + (simplify +(cnd (cmp @0 zerop) (convert?@2 @0) (negate@1 @2)) +(if (cmp == EQ_EXPR || cmp == UNEQ_EXPR) + @1) +(if (cmp == NE_EXPR || cmp == LTGT_EXPR) + (non_lvalue @2)) +(if (TYPE_SIGN (TREE_TYPE (@0)) == SIGNED /* implicit */ + TYPE_SIGN (type) == SIGNED + element_precision (type) = element_precision (TREE_TYPE (@0))) + (if (cmp == GE_EXPR || cmp == GT_EXPR + || (!flag_trapping_math (cmp == UNGE_EXPR || cmp == UNGT_EXPR))) + (abs @2)) + (if (cmp == LE_EXPR || cmp == LT_EXPR + || (!flag_trapping_math (cmp == UNLE_EXPR || cmp == UNLT_EXPR))) + (negate (abs @2) + /* Now with the branches swapped. */ + (simplify +(cnd (cmp @0 zerop) (negate@1 (convert?@2 @0)) @2) not obvious from a quick look - but would you be able to remove the swapped branch vairant if (cnd:c (cmp @0 zerop) X Y) would work by swapping X and Y? Hmm. How do I test if I am currently in the original or commuted version of the simplification? You can't. I could add a with block that defines truecmp as either cmp or invert_tree_comparison (cmp) and test that. Otherwise, I would need a test before each return as swapped versions don't return the same thing. It might make a slight difference on the handling of flag_trapping_math, but that handling already seems strange to me... (cnd:c (cmp @0 zerop) (convert?@2 @0) (negate@1 @2)) would get you (cnd (cmp @0 zerop) (convert?@2 @0) (negate@1 @2)) and (cnd (cmp @0 zerop) (negate@1 @2) (convert?@2 @0)) in the patterns it almost literally looked like what you did manually. The fold-const.c code doesn't seem to handle as many variants (esp. the swapping?), The fold-const.c function is called twice, once on regular operands, once with inverted comparison and swapped operands. I really don't think I am handling more cases (except maybe the silly a?a:0 is extended to unsigned). Ok. so maybe you can add a testcase that exercises some of the above on GIMPLE? So mostly the VEC_COND_EXPR version? We don't seem to have that much COND_EXPR left in gimple. Ah, true. Yes, the vector variant then. Thanks, Richard. -- Marc Glisse
Re: Move ABS detection from fold-const.c to match.pd
(this message looks like it was lost in my draft folder...) On Tue, 26 May 2015, Richard Biener wrote: +(match zerop integer_zerop) +(match zerop real_zerop) Would it also include fixed_zerop? Probably, yes. The main issue is that I know next to nothing about fixed-point types, so I am always unsure how to handle them (when I don't forget them completely). For instance, in the recently added -A CMP -B, we could probably replace (if (FLOAT_TYPE_P (TREE_TYPE (@0)) || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0)) TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0 with (if (FLOAT_TYPE_P (TREE_TYPE (@0)) || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))) Note that with inlining implemented it would duplicate the pattern for each match variant thus in this case adding a tree.[ch] function zerop () might be better. Ah... I actually thought we might end up moving things like integer_zerop from tree.c to match.pd, especially since predicates are not declared 'static'... Ok, reverse gear. Note that inlining does not seem necessary to implement more advanced predicates like negated_value_for_comparison in the parent message. + (simplify +(cnd (cmp @0 zerop) (convert?@2 @0) (negate@1 @2)) +(if (cmp == EQ_EXPR || cmp == UNEQ_EXPR) + @1) +(if (cmp == NE_EXPR || cmp == LTGT_EXPR) + (non_lvalue @2)) +(if (TYPE_SIGN (TREE_TYPE (@0)) == SIGNED /* implicit */ + TYPE_SIGN (type) == SIGNED + element_precision (type) = element_precision (TREE_TYPE (@0))) + (if (cmp == GE_EXPR || cmp == GT_EXPR + || (!flag_trapping_math (cmp == UNGE_EXPR || cmp == UNGT_EXPR))) + (abs @2)) + (if (cmp == LE_EXPR || cmp == LT_EXPR + || (!flag_trapping_math (cmp == UNLE_EXPR || cmp == UNLT_EXPR))) + (negate (abs @2) + /* Now with the branches swapped. */ + (simplify +(cnd (cmp @0 zerop) (negate@1 (convert?@2 @0)) @2) not obvious from a quick look - but would you be able to remove the swapped branch vairant if (cnd:c (cmp @0 zerop) X Y) would work by swapping X and Y? Hmm. How do I test if I am currently in the original or commuted version of the simplification? I could add a with block that defines truecmp as either cmp or invert_tree_comparison (cmp) and test that. Otherwise, I would need a test before each return as swapped versions don't return the same thing. It might make a slight difference on the handling of flag_trapping_math, but that handling already seems strange to me... The fold-const.c code doesn't seem to handle as many variants (esp. the swapping?), The fold-const.c function is called twice, once on regular operands, once with inverted comparison and swapped operands. I really don't think I am handling more cases (except maybe the silly a?a:0 is extended to unsigned). so maybe you can add a testcase that exercises some of the above on GIMPLE? So mostly the VEC_COND_EXPR version? We don't seem to have that much COND_EXPR left in gimple. -- Marc Glisse
Re: Move ABS detection from fold-const.c to match.pd
On Sun, May 24, 2015 at 3:17 PM, Marc Glisse marc.gli...@inria.fr wrote: I forgot to mention I optimistically tried to write something like this: (match (negated_value_for_comparison @0) (negate @0)) (match (negated_value_for_comparison (negate @0)) @0) (match (negated_value_for_comparison (minus @0 @1)) (if (!HONOR_SIGN_DEPENDENT_ROUNDING (type)) (minus @1 @0)) without success. There is already a comment for logical_inverted_value about related limitations in genmatch. Yeah, Prathamesh was working on inlining - not sure if that ended up in sth usable? +(match zerop integer_zerop) +(match zerop real_zerop) Would it also include fixed_zerop? Note that with inlining implemented it would duplicate the pattern for each match variant thus in this case adding a tree.[ch] function zerop () might be better. + (simplify +(cnd (cmp @0 zerop) (convert?@2 @0) (negate@1 @2)) +(if (cmp == EQ_EXPR || cmp == UNEQ_EXPR) + @1) +(if (cmp == NE_EXPR || cmp == LTGT_EXPR) + (non_lvalue @2)) +(if (TYPE_SIGN (TREE_TYPE (@0)) == SIGNED /* implicit */ + TYPE_SIGN (type) == SIGNED + element_precision (type) = element_precision (TREE_TYPE (@0))) + (if (cmp == GE_EXPR || cmp == GT_EXPR + || (!flag_trapping_math (cmp == UNGE_EXPR || cmp == UNGT_EXPR))) + (abs @2)) + (if (cmp == LE_EXPR || cmp == LT_EXPR + || (!flag_trapping_math (cmp == UNLE_EXPR || cmp == UNLT_EXPR))) + (negate (abs @2) + /* Now with the branches swapped. */ + (simplify +(cnd (cmp @0 zerop) (negate@1 (convert?@2 @0)) @2) not obvious from a quick look - but would you be able to remove the swapped branch vairant if (cnd:c (cmp @0 zerop) X Y) would work by swapping X and Y? The fold-const.c code doesn't seem to handle as many variants (esp. the swapping?), so maybe you can add a testcase that exercises some of the above on GIMPLE? Thanks, Richard. -- Marc Glisse
Re: Move ABS detection from fold-const.c to match.pd
I forgot to mention I optimistically tried to write something like this: (match (negated_value_for_comparison @0) (negate @0)) (match (negated_value_for_comparison (negate @0)) @0) (match (negated_value_for_comparison (minus @0 @1)) (if (!HONOR_SIGN_DEPENDENT_ROUNDING (type)) (minus @1 @0)) without success. There is already a comment for logical_inverted_value about related limitations in genmatch. -- Marc Glisse
Move ABS detection from fold-const.c to match.pd
Hello,
I don't think this pattern is done in the branch. Here I am trying to
match what is done in fold-const, but the idea is that we can later add an
extra block where we replace (cmp (minus @2 @3) zerop) with (cmp @2 @3),
maybe with some adjustments (convert? all over the place), to help with
PR 64450 / PR 61734.
I didn't exactly match the code in fold-const.c, among other things
because I didn't feel like calling operand_equal_for_comparison_p, and
strip_nops can translate to quite heavy patterns. Except for a few
irrelevant cases (where I simplify more), this seems to generate the same
.original dumps in the cases I tried. But I am not claiming this is the
best way to arrange that code, better ideas are welcome.
I don't understand the old code handling unsigned, but I couldn't find a
case where the new code (doing nothing special for unsigned) generated a
different .original dump. The last pattern had a strange mix of requiring
integer_zerop and talking about signed zero and NaN, I didn't try to
preserve that. The change to genmatch is for zerop, which doesn't try to
valueize anything.
I had a few issues with the machinery. First, genmatch was generating
a switch with duplicate cases (miraculously, replacing 'cond' with an
iteration on 'cnd' worked around it). Second, (plus @0 (negate@0 @1)) is
treated as (plus @0 @0), the pattern on the second occurence of the
capture is silently ignored.
Regtested on ppc64le-redhat-linux.
2015-05-25 Marc Glisse marc.gli...@inria.fr
* genmatch.c (write_predicate): Add ATTRIBUTE_UNUSED.
* match.pd (A op 0 ? A : -A, A op 0 ? A : 0): New simplifications.
* fold-const.c (fold_cond_expr_with_comparison): Remove
corresponding code.
--
Marc GlisseIndex: gcc/fold-const.c
===
--- gcc/fold-const.c(revision 223630)
+++ gcc/fold-const.c(working copy)
@@ -4875,112 +4875,25 @@ merge_ranges (int *pin_p, tree *plow, tr
Return a folded expression whose code is not a COND_EXPR
anymore, or NULL_TREE if no folding opportunity is found. */
static tree
fold_cond_expr_with_comparison (location_t loc, tree type,
tree arg0, tree arg1, tree arg2)
{
enum tree_code comp_code = TREE_CODE (arg0);
tree arg00 = TREE_OPERAND (arg0, 0);
tree arg01 = TREE_OPERAND (arg0, 1);
- tree arg1_type = TREE_TYPE (arg1);
tree tem;
STRIP_NOPS (arg1);
STRIP_NOPS (arg2);
- /* If we have A op 0 ? A : -A, consider applying the following
- transformations:
-
- A == 0? A : -Asame as -A
- A != 0? A : -Asame as A
- A = 0? A : -Asame as abs (A)
- A 0? A : -Asame as abs (A)
- A = 0? A : -Asame as -abs (A)
- A 0? A : -Asame as -abs (A)
-
- None of these transformations work for modes with signed
- zeros. If A is +/-0, the first two transformations will
- change the sign of the result (from +0 to -0, or vice
- versa). The last four will fix the sign of the result,
- even though the original expressions could be positive or
- negative, depending on the sign of A.
-
- Note that all these transformations are correct if A is
- NaN, since the two alternatives (A and -A) are also NaNs. */
- if (!HONOR_SIGNED_ZEROS (element_mode (type))
- (FLOAT_TYPE_P (TREE_TYPE (arg01))
- ? real_zerop (arg01)
- : integer_zerop (arg01))
- ((TREE_CODE (arg2) == NEGATE_EXPR
- operand_equal_p (TREE_OPERAND (arg2, 0), arg1, 0))
-/* In the case that A is of the form X-Y, '-A' (arg2) may
- have already been folded to Y-X, check for that. */
- || (TREE_CODE (arg1) == MINUS_EXPR
- TREE_CODE (arg2) == MINUS_EXPR
- operand_equal_p (TREE_OPERAND (arg1, 0),
- TREE_OPERAND (arg2, 1), 0)
- operand_equal_p (TREE_OPERAND (arg1, 1),
- TREE_OPERAND (arg2, 0), 0
-switch (comp_code)
- {
- case EQ_EXPR:
- case UNEQ_EXPR:
- tem = fold_convert_loc (loc, arg1_type, arg1);
- return pedantic_non_lvalue_loc (loc,
- fold_convert_loc (loc, type,
- negate_expr (tem)));
- case NE_EXPR:
- case LTGT_EXPR:
- return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type,
arg1));
- case UNGE_EXPR:
- case UNGT_EXPR:
- if (flag_trapping_math)
- break;
- /* Fall through. */
- case GE_EXPR:
- case GT_EXPR:
- if (TYPE_UNSIGNED (TREE_TYPE (arg1)))
- arg1 = fold_convert_loc (loc, signed_type_for
- (TREE_TYPE (arg1)), arg1);
- tem = fold_build1_loc (loc, ABS_EXPR, TREE_TYPE (arg1), arg1);
- return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, tem));
- case UNLE_EXPR:
- case
