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
