| Message ID | alpine.DEB.2.02.1505231828530.22033@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
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.
On Sun, May 24, 2015 at 3:17 PM, Marc Glisse <marc.glisse@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
(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.
On Sun, Jun 28, 2015 at 8:34 PM, Marc Glisse <marc.glisse@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
Index: 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 : -A same as -A - A != 0? A : -A same as A - A >= 0? A : -A same as abs (A) - A > 0? A : -A same as abs (A) - A <= 0? A : -A same as -abs (A) - A < 0? A : -A same 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 UNLT_EXPR: - if (flag_trapping_math) - break; - case LE_EXPR: - case LT_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 negate_expr (fold_convert_loc (loc, type, tem)); - default: - gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison); - break; - } - - /* A != 0 ? A : 0 is simply A, unless A is -0. Likewise - A == 0 ? A : 0 is always 0 unless A is -0. Note that - both transformations are correct when A is NaN: A != 0 - is then true, and A == 0 is false. */ - - if (!HONOR_SIGNED_ZEROS (element_mode (type)) - && integer_zerop (arg01) && integer_zerop (arg2)) - { - if (comp_code == NE_EXPR) - return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, arg1)); - else if (comp_code == EQ_EXPR) - return build_zero_cst (type); - } - /* Try some transformations of A op B ? A : B. A == B? A : B same as B A != B? A : B same as A A >= B? A : B same as max (A, B) A > B? A : B same as max (B, A) A <= B? A : B same as min (A, B) A < B? A : B same as min (B, A) As above, these transformations don't work in the presence Index: gcc/genmatch.c =================================================================== --- gcc/genmatch.c (revision 223630) +++ gcc/genmatch.c (working copy) @@ -2657,21 +2657,21 @@ decision_tree::gen_generic (FILE *f) /* Output code to implement the predicate P from the decision tree DT. */ void write_predicate (FILE *f, predicate_id *p, decision_tree &dt, bool gimple) { fprintf (f, "\nbool\n" "%s%s (tree t%s%s)\n" "{\n", gimple ? "gimple_" : "tree_", p->id, p->nargs > 0 ? ", tree *res_ops" : "", - gimple ? ", tree (*valueize)(tree)" : ""); + gimple ? ", tree (*valueize)(tree) ATTRIBUTE_UNUSED" : ""); /* Conveniently make 'type' available. */ fprintf (f, "tree type = TREE_TYPE (t);\n"); if (!gimple) fprintf (f, "if (TREE_SIDE_EFFECTS (t)) return false;\n"); dt.root->gen_kids (f, gimple); fprintf (f, "return false;\n" "}\n"); } Index: gcc/match.pd =================================================================== --- gcc/match.pd (revision 223630) +++ gcc/match.pd (working copy) @@ -1131,10 +1131,106 @@ along with GCC; see the file COPYING3. && (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)) || tree_int_cst_sgn (@4) >= 0) && single_use (@5)) (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))) (with { tree ntype = TREE_TYPE (@0); } (convert (bit_and (op @0 @1) (convert:ntype @4))))) (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } (convert (bit_and (op (convert:utype @0) (convert:utype @1)) (convert:utype @4))))))) + +(match zerop integer_zerop) +(match zerop real_zerop) + +/* If we have A op 0 ? A : -A, consider applying the following + transformations: + + A == 0? A : -A same as -A + A != 0? A : -A same as A + A >= 0? A : -A same as abs (A) + A > 0? A : -A same as abs (A) + A <= 0? A : -A same as -abs (A) + A < 0? A : -A same 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 (type)) + (for cnd (cond vec_cond) + (for cmp (tcc_comparison) + (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) + (if (cmp == EQ_EXPR || cmp == UNEQ_EXPR) + (non_lvalue @2)) + (if (cmp == NE_EXPR || cmp == LTGT_EXPR) + @1) + (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))) + (negate (abs @2))) + (if (cmp == LE_EXPR || cmp == LT_EXPR + || (!flag_trapping_math && (cmp == UNLE_EXPR || cmp == UNLT_EXPR))) + (abs @2)))) + + /* Same as above, but if A is X - Y, -A may be spelled Y - X. */ + (simplify + (cnd (cmp (minus@0 @2 @3) zerop) @0 (minus@1 @3 @2)) + (if (cmp == EQ_EXPR || cmp == UNEQ_EXPR) + @1) + (if (cmp == NE_EXPR || cmp == LTGT_EXPR) + @0) + (if (!HONOR_SIGN_DEPENDENT_ROUNDING (type)) + (if (cmp == GE_EXPR || cmp == GT_EXPR + || (!flag_trapping_math && (cmp == UNGE_EXPR || cmp == UNGT_EXPR))) + (abs @0)) + (if (cmp == LE_EXPR || cmp == LT_EXPR + || (!flag_trapping_math && (cmp == UNLE_EXPR || cmp == UNLT_EXPR))) + (negate (abs @0))))) + (simplify + (cnd (cmp (minus@0 @2 @3) zerop) (minus@1 @3 @2) @0) + (if (cmp == EQ_EXPR || cmp == UNEQ_EXPR) + @0) + (if (cmp == NE_EXPR || cmp == LTGT_EXPR) + @1) + (if (!HONOR_SIGN_DEPENDENT_ROUNDING (type)) + (if (cmp == GE_EXPR || cmp == GT_EXPR + || (!flag_trapping_math && (cmp == UNGE_EXPR || cmp == UNGT_EXPR))) + (negate (abs @0))) + (if (cmp == LE_EXPR || cmp == LT_EXPR + || (!flag_trapping_math && (cmp == UNLE_EXPR || cmp == UNLT_EXPR))) + (abs @0))))) + + /* A != 0 ? A : 0 is simply A, unless A is -0. Likewise + A == 0 ? A : 0 is always 0 unless A is -0. Note that + both transformations are correct when A is NaN: A != 0 + is then true, and A == 0 is false. */ + (simplify + (cnd (ne @0 zerop) (convert? @0) zerop) + (non_lvalue (convert @0))) + (simplify + (cnd (eq @0 zerop) (convert? @0) zerop@1) + (non_lvalue (convert @1)))))
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.glisse@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.