On Fri, 6 Dec 2019, Richard Biener wrote: > On Fri, 6 Dec 2019, Marc Glisse wrote: > > > On Fri, 6 Dec 2019, Richard Biener wrote: > > > > >>> nop_convert sees that 'a' comes from a conversion, and only returns d > > >>> (unlike 'convert?' which would try both a and d). > > > > Maybe I should have formulated it as: nop_convert works kind of like a > > strip_nops. > > > > >> If use gets more and more we can make nop_convert a first class citizen > > >> and > > >> allow a? Variant. > > > > One reason I did not specifically push for that is that nop_convert is > > seldom > > the right condition. It is convenient because it is usually easy enough to > > check that it is correct, but in most cases one of narrowing / > > zero-extension > > / sign-extension also works. Still, it is better to handle just NOPs than no > > conversion at all, so I guess making that easy is still good. > > In my view nop_convert is useful to avoid cluttering the code with > (if (tree_nop_conversion_p ...) checks that are even redundant > when a (convert? ... is stripped away. > > > > Like the attached (need to adjust docs & rename some functions still) > > > which generalizes > > > [digit]? parsing. This allows you to write (nop_convert? ...) > > > > I guess once this is in, we should replace all (most?) 'nop_convert' with > > 'nop_convert?' (and possibly a digit in some places) and remove the last > > alternative in the definition of nop_convert. > > Yes, that was my thinking. > > > Although that will increase the code size. In case, we could still have > > both a > > nop_convert and a strip_nop predicates. Just thinking: > > We should measure it, yes, I hope it won't be too bad ;) In theory > making genmatch emitted code more like a graph rather than a tree > (with shared leafs) might save us a bit here. > > > (match (nop_convert @0) > > (convert @0) > > (if (tree_nop_conversion_p (type, TREE_TYPE (@0))))) > > (match (nop_convert @0) > > (view_convert @0) > > (if (VECTOR_TYPE_P (type) && VECTOR_TYPE_P (TREE_TYPE (@0)) > > && known_eq (TYPE_VECTOR_SUBPARTS (type), > > TYPE_VECTOR_SUBPARTS (TREE_TYPE (@0))) > > && tree_nop_conversion_p (TREE_TYPE (type), TREE_TYPE (TREE_TYPE > > && (@0)))))) > > > > (match (strip_nops @0) > > (nop_convert? @0)) > > > > but that relies on the fact that when there is an optional operation, the > > machinery first tries with the operation, and then without, the order > > matters. > > Maybe being explicit on the priorities is safer > > > > (match (strip_nops @0) > > (nop_convert @0)) > > (match (strip_nops @0) > > @0) > > Yeah, I don't think the above complexity is worth it. > > > > It works (generated files are unchanged), so I guess I'll push it > > > after polishing. > > > > > > It also extends the 1/2/3 grouping to be able to group like (plus > > > (nop_convert2? @0) (convert2? @1)) > > > so either both will be present or none (meaning that when grouping you > > > can choose the "cheaper" > > > test for one in case you know the conversions will be the same). > > > > Nice. > > r279037.
And testing the following now, replacing all nop_converts. > wc -l gimple-match.c.orig 117182 gimple-match.c.orig > wc -l gimple-match.c 119052 gimple-match.c so impact is minimal. Richard. 2019-12-06 Richard Biener <rguent...@suse.de> * match.pd (nop_convert): Remove empty match. Use nop_convert? everywhere. Index: gcc/match.pd =================================================================== --- gcc/match.pd (revision 279037) +++ gcc/match.pd (working copy) @@ -98,8 +98,8 @@ (define_operator_list UNCOND_TERNARY (define_operator_list COND_TERNARY IFN_COND_FMA IFN_COND_FMS IFN_COND_FNMA IFN_COND_FNMS) -/* As opposed to convert?, this still creates a single pattern, so - it is not a suitable replacement for convert? in all cases. */ +/* With nop_convert? combine convert? and view_convert? in one pattern + plus conditionalize on tree_nop_conversion_p conversions. */ (match (nop_convert @0) (convert @0) (if (tree_nop_conversion_p (type, TREE_TYPE (@0))))) @@ -109,9 +109,6 @@ (define_operator_list COND_TERNARY && known_eq (TYPE_VECTOR_SUBPARTS (type), TYPE_VECTOR_SUBPARTS (TREE_TYPE (@0))) && tree_nop_conversion_p (TREE_TYPE (type), TREE_TYPE (TREE_TYPE (@0)))))) -/* This one has to be last, or it shadows the others. */ -(match (nop_convert @0) - @0) /* Transform likes of (char) ABS_EXPR <(int) x> into (char) ABSU_EXPR <x> ABSU_EXPR returns unsigned absolute value of the operand and the operand @@ -1428,7 +1425,7 @@ (define_operator_list COND_TERNARY /* Convert - (~A) to A + 1. */ (simplify - (negate (nop_convert (bit_not @0))) + (negate (nop_convert? (bit_not @0))) (plus (view_convert @0) { build_each_one_cst (type); })) /* Convert ~ (A - 1) or ~ (A + -1) to -A. */ @@ -1455,7 +1452,7 @@ (define_operator_list COND_TERNARY /* Otherwise prefer ~(X ^ Y) to ~X ^ Y as more canonical. */ (simplify - (bit_xor:c (nop_convert:s (bit_not:s @0)) @1) + (bit_xor:c (nop_convert?:s (bit_not:s @0)) @1) (if (tree_nop_conversion_p (type, TREE_TYPE (@0))) (bit_not (bit_xor (view_convert @0) @1)))) @@ -1684,7 +1681,7 @@ (define_operator_list COND_TERNARY /* For equality, this is also true with wrapping overflow. */ (for op (eq ne) (simplify - (op:c (nop_convert@3 (plus:c@2 @0 (convert1? @1))) (convert2? @1)) + (op:c (nop_convert?@3 (plus:c@2 @0 (convert1? @1))) (convert2? @1)) (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0)) && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)) || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))) @@ -1693,7 +1690,7 @@ (define_operator_list COND_TERNARY && tree_nop_conversion_p (TREE_TYPE (@3), TREE_TYPE (@1))) (op @0 { build_zero_cst (TREE_TYPE (@0)); }))) (simplify - (op:c (nop_convert@3 (pointer_plus@2 (convert1? @0) @1)) (convert2? @0)) + (op:c (nop_convert?@3 (pointer_plus@2 (convert1? @0) @1)) (convert2? @0)) (if (tree_nop_conversion_p (TREE_TYPE (@2), TREE_TYPE (@0)) && tree_nop_conversion_p (TREE_TYPE (@3), TREE_TYPE (@0)) && (CONSTANT_CLASS_P (@1) || (single_use (@2) && single_use (@3)))) @@ -2142,7 +2139,7 @@ (define_operator_list COND_TERNARY || !HONOR_SIGN_DEPENDENT_ROUNDING (type))) (convert (negate @1)))) (simplify - (negate (nop_convert (negate @1))) + (negate (nop_convert? (negate @1))) (if (!TYPE_OVERFLOW_SANITIZED (type) && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@1))) (view_convert @1))) @@ -2159,25 +2156,25 @@ (define_operator_list COND_TERNARY /* A - (A +- B) -> -+ B */ /* A +- (B -+ A) -> +- B */ (simplify - (minus (nop_convert (plus:c (nop_convert @0) @1)) @0) + (minus (nop_convert1? (plus:c (nop_convert2? @0) @1)) @0) (view_convert @1)) (simplify - (minus (nop_convert (minus (nop_convert @0) @1)) @0) + (minus (nop_convert1? (minus (nop_convert2? @0) @1)) @0) (if (!ANY_INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_WRAPS (type)) (negate (view_convert @1)) (view_convert (negate @1)))) (simplify - (plus:c (nop_convert (minus @0 (nop_convert @1))) @1) + (plus:c (nop_convert1? (minus @0 (nop_convert2? @1))) @1) (view_convert @0)) (simplify - (minus @0 (nop_convert (plus:c (nop_convert @0) @1))) + (minus @0 (nop_convert1? (plus:c (nop_convert2? @0) @1))) (if (!ANY_INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_WRAPS (type)) (negate (view_convert @1)) (view_convert (negate @1)))) (simplify - (minus @0 (nop_convert (minus (nop_convert @0) @1))) + (minus @0 (nop_convert1? (minus (nop_convert2? @0) @1))) (view_convert @1)) /* (A +- B) + (C - A) -> C +- B */ /* (A + B) - (A - C) -> B + C */ @@ -2204,7 +2201,7 @@ (define_operator_list COND_TERNARY (for inner_op (plus minus) neg_inner_op (minus plus) (simplify - (outer_op (nop_convert (inner_op @0 CONSTANT_CLASS_P@1)) + (outer_op (nop_convert? (inner_op @0 CONSTANT_CLASS_P@1)) CONSTANT_CLASS_P@2) /* If one of the types wraps, use that one. */ (if (!ANY_INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_WRAPS (type)) @@ -2243,7 +2240,7 @@ (define_operator_list COND_TERNARY /* (CST1 - A) +- CST2 -> CST3 - A */ (for outer_op (plus minus) (simplify - (outer_op (nop_convert (minus CONSTANT_CLASS_P@1 @0)) CONSTANT_CLASS_P@2) + (outer_op (nop_convert? (minus CONSTANT_CLASS_P@1 @0)) CONSTANT_CLASS_P@2) /* If one of the types wraps, use that one. */ (if (!ANY_INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_WRAPS (type)) /* If all 3 captures are CONSTANT_CLASS_P, punt, as we might recurse @@ -2262,7 +2259,7 @@ (define_operator_list COND_TERNARY Use view_convert because it is safe for vectors and equivalent for scalars. */ (simplify - (minus CONSTANT_CLASS_P@1 (nop_convert (minus CONSTANT_CLASS_P@2 @0))) + (minus CONSTANT_CLASS_P@1 (nop_convert? (minus CONSTANT_CLASS_P@2 @0))) /* If one of the types wraps, use that one. */ (if (!ANY_INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_WRAPS (type)) /* If all 3 captures are CONSTANT_CLASS_P, punt, as we might recurse