On 9/11/19 3:19 PM, Martin Liška wrote:
> On 9/11/19 2:43 PM, Richard Biener wrote:
>> On Wed, 11 Sep 2019, Martin Liška wrote:
>>
>>> I'm sending updated version of the patch where I changed
>>> from previous version:
>>> - more compact matching is used (note @3 and @4):
>>> (and (code1:c@3 @0 INTEGER_CST@1) (code2:c@4 @0 INTEGER_CST@2))
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Ready to be installed?
>>
>> --- a/gcc/genmatch.c
>> +++ b/gcc/genmatch.c
>> @@ -1894,9 +1894,11 @@ dt_node *
>> dt_node::append_simplify (simplify *s, unsigned pattern_no,
>> dt_operand **indexes)
>> {
>> + dt_simplify *s2;
>> dt_simplify *n = new dt_simplify (s, pattern_no, indexes);
>> for (unsigned i = 0; i < kids.length (); ++i)
>> - if (dt_simplify *s2 = dyn_cast <dt_simplify *> (kids[i]))
>> + if ((s2 = dyn_cast <dt_simplify *> (kids[i]))
>> + && s->match->location != s2->s->match->location)
>> {
>>
>> can you retain the warning for verbose >= 1 please? And put in
>> a comment that duplicates are sometimes hard to avoid with
>> nested for so this keeps match.pd sources small.
>
> Sure.
>
>>
>> + /* Function maybe_fold_comparisons_from_match_pd creates temporary
>> + SSA_NAMEs. */
>> + if (TREE_CODE (op1) == SSA_NAME && TREE_CODE (op2) == SSA_NAME)
>> + {
>> + gimple *s = SSA_NAME_DEF_STMT (op2);
>> + if (is_gimple_assign (s))
>> + return same_bool_comparison_p (op1, gimple_assign_rhs_code (s),
>> + gimple_assign_rhs1 (s),
>> + gimple_assign_rhs2 (s));
>> + else
>> + return false;
>> + }
>>
>> when do you actually run into the need to add this? The whole
>> same_bool_result_p/same_bool_comparison_p helpers look a bit
>> odd to me. It shouldn't be special to the new temporaries
>> either so at least the comment looks out-of place.
>
> At some point it was needed to handle gcc/testsuite/gcc.dg/pr46909.c
> test-case. Apparently, now the test-case is fine with the hunk. I will it
> removal of the hunk.
So it's not needed.
>
>>
>> + else if (op.code.is_tree_code ()
>> + && TREE_CODE_CLASS ((tree_code)op.code) == tcc_comparison)
>> + {
>>
>> COMPARISON_CLASS_P ((tree_code)op.code)
>>
>> as was noted.
>
> Won't work here:
>
> /home/marxin/Programming/gcc/gcc/gimple-fold.c: In function ‘tree_node*
> maybe_fold_comparisons_from_match_pd(tree, tree_code, tree_code, tree, tree,
> tree_code, tree, tree)’:
> /home/marxin/Programming/gcc/gcc/tree.h:239:49: error: base operand of ‘->’
> is not a pointer
> 239 | #define TREE_CODE(NODE) ((enum tree_code) (NODE)->base.code)
> | ^~
>
>>
>> Any particular reason you needed to swap the calls in
>> maybe_fold_and/or_comparisons?
>>
>> +(for code1 (eq ne)
>> + (for code2 (eq ne lt gt le ge)
>> + (for and (truth_and bit_and)
>> + (simplify
>>
>> You could save some code-bloat with writing
>>
>> (for and (
>> #if GENERIC
>> truth_and
>> #endif
>> bit_and)
>>
>> but since you are moving the patterns from GIMPLE code I'd say
>> simply remove truth_and and that innermost for?
>
> I'm gonna drop the generic tree codes support.
It's dropped in the updated patch.
Martin
>
> Martin
>
>>
>> Otherwise OK.
>>
>> Thanks,
>> Richard.
>>
>>
>>
>>
>>> Thanks,
>>> Martin
>>>
>>
>
>From f4e6cf9aec14111a35b1c8d641f83ec355d8c7e0 Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Fri, 6 Sep 2019 12:34:49 +0200
Subject: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
gcc/ChangeLog:
2019-09-09 Martin Liska <mli...@suse.cz>
* genmatch.c (dt_node::append_simplify): Do not print
warning when we have duplicate patterns belonging
to a same simplify rule.
* gimple-fold.c (and_comparisons_1): Remove matching moved to match.pd.
(maybe_fold_comparisons_from_match_pd): Handle
tcc_comparison as a results.
(maybe_fold_and_comparisons):Call maybe_fold_comparisons_from_match_pd
first.
(maybe_fold_or_comparisons): Likewise.
* match.pd: Handle (X == CST1) && (X OP2 CST2) conditions.
---
gcc/genmatch.c | 7 +-
gcc/gimple-fold.c | 161 ++++++----------------------------------------
gcc/match.pd | 66 +++++++++++++++++++
3 files changed, 93 insertions(+), 141 deletions(-)
diff --git a/gcc/genmatch.c b/gcc/genmatch.c
index 2e7bf27eeda..cede432cdc9 100644
--- a/gcc/genmatch.c
+++ b/gcc/genmatch.c
@@ -1894,10 +1894,15 @@ dt_node *
dt_node::append_simplify (simplify *s, unsigned pattern_no,
dt_operand **indexes)
{
+ dt_simplify *s2;
dt_simplify *n = new dt_simplify (s, pattern_no, indexes);
for (unsigned i = 0; i < kids.length (); ++i)
- if (dt_simplify *s2 = dyn_cast <dt_simplify *> (kids[i]))
+ if ((s2 = dyn_cast <dt_simplify *> (kids[i]))
+ && (verbose >= 1
+ || s->match->location != s2->s->match->location))
{
+ /* With a nested patters, it's hard to avoid these in order
+ to keep match.pd rules relatively small. */
warning_at (s->match->location, "duplicate pattern");
warning_at (s2->s->match->location, "previous pattern defined here");
print_operand (s->match, stderr);
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 6d9ba367839..23891fed930 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -5620,136 +5620,6 @@ and_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b,
return t;
}
- /* If both comparisons are of the same value against constants, we might
- be able to merge them. */
- if (operand_equal_p (op1a, op2a, 0)
- && TREE_CODE (op1b) == INTEGER_CST
- && TREE_CODE (op2b) == INTEGER_CST)
- {
- int cmp = tree_int_cst_compare (op1b, op2b);
-
- /* If we have (op1a == op1b), we should either be able to
- return that or FALSE, depending on whether the constant op1b
- also satisfies the other comparison against op2b. */
- if (code1 == EQ_EXPR)
- {
- bool done = true;
- bool val;
- switch (code2)
- {
- case EQ_EXPR: val = (cmp == 0); break;
- case NE_EXPR: val = (cmp != 0); break;
- case LT_EXPR: val = (cmp < 0); break;
- case GT_EXPR: val = (cmp > 0); break;
- case LE_EXPR: val = (cmp <= 0); break;
- case GE_EXPR: val = (cmp >= 0); break;
- default: done = false;
- }
- if (done)
- {
- if (val)
- return fold_build2 (code1, boolean_type_node, op1a, op1b);
- else
- return boolean_false_node;
- }
- }
- /* Likewise if the second comparison is an == comparison. */
- else if (code2 == EQ_EXPR)
- {
- bool done = true;
- bool val;
- switch (code1)
- {
- case EQ_EXPR: val = (cmp == 0); break;
- case NE_EXPR: val = (cmp != 0); break;
- case LT_EXPR: val = (cmp > 0); break;
- case GT_EXPR: val = (cmp < 0); break;
- case LE_EXPR: val = (cmp >= 0); break;
- case GE_EXPR: val = (cmp <= 0); break;
- default: done = false;
- }
- if (done)
- {
- if (val)
- return fold_build2 (code2, boolean_type_node, op2a, op2b);
- else
- return boolean_false_node;
- }
- }
-
- /* Same business with inequality tests. */
- else if (code1 == NE_EXPR)
- {
- bool val;
- switch (code2)
- {
- case EQ_EXPR: val = (cmp != 0); break;
- case NE_EXPR: val = (cmp == 0); break;
- case LT_EXPR: val = (cmp >= 0); break;
- case GT_EXPR: val = (cmp <= 0); break;
- case LE_EXPR: val = (cmp > 0); break;
- case GE_EXPR: val = (cmp < 0); break;
- default:
- val = false;
- }
- if (val)
- return fold_build2 (code2, boolean_type_node, op2a, op2b);
- }
- else if (code2 == NE_EXPR)
- {
- bool val;
- switch (code1)
- {
- case EQ_EXPR: val = (cmp == 0); break;
- case NE_EXPR: val = (cmp != 0); break;
- case LT_EXPR: val = (cmp <= 0); break;
- case GT_EXPR: val = (cmp >= 0); break;
- case LE_EXPR: val = (cmp < 0); break;
- case GE_EXPR: val = (cmp > 0); break;
- default:
- val = false;
- }
- if (val)
- return fold_build2 (code1, boolean_type_node, op1a, op1b);
- }
-
- /* Chose the more restrictive of two < or <= comparisons. */
- else if ((code1 == LT_EXPR || code1 == LE_EXPR)
- && (code2 == LT_EXPR || code2 == LE_EXPR))
- {
- if ((cmp < 0) || (cmp == 0 && code1 == LT_EXPR))
- return fold_build2 (code1, boolean_type_node, op1a, op1b);
- else
- return fold_build2 (code2, boolean_type_node, op2a, op2b);
- }
-
- /* Likewise chose the more restrictive of two > or >= comparisons. */
- else if ((code1 == GT_EXPR || code1 == GE_EXPR)
- && (code2 == GT_EXPR || code2 == GE_EXPR))
- {
- if ((cmp > 0) || (cmp == 0 && code1 == GT_EXPR))
- return fold_build2 (code1, boolean_type_node, op1a, op1b);
- else
- return fold_build2 (code2, boolean_type_node, op2a, op2b);
- }
-
- /* Check for singleton ranges. */
- else if (cmp == 0
- && ((code1 == LE_EXPR && code2 == GE_EXPR)
- || (code1 == GE_EXPR && code2 == LE_EXPR)))
- return fold_build2 (EQ_EXPR, boolean_type_node, op1a, op2b);
-
- /* Check for disjoint ranges. */
- else if (cmp <= 0
- && (code1 == LT_EXPR || code1 == LE_EXPR)
- && (code2 == GT_EXPR || code2 == GE_EXPR))
- return boolean_false_node;
- else if (cmp >= 0
- && (code1 == GT_EXPR || code1 == GE_EXPR)
- && (code2 == LT_EXPR || code2 == LE_EXPR))
- return boolean_false_node;
- }
-
/* Perhaps the first comparison is (NAME != 0) or (NAME == 1) where
NAME's definition is a truth value. See if there are any simplifications
that can be done against the NAME's definition. */
@@ -5899,6 +5769,16 @@ maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code,
else
return res;
}
+ else if (op.code.is_tree_code ()
+ && TREE_CODE_CLASS ((tree_code)op.code) == tcc_comparison)
+ {
+ tree op0 = op.ops[0];
+ tree op1 = op.ops[1];
+ if (op0 == lhs1 || op0 == lhs2 || op1 == lhs1 || op1 == lhs2)
+ return NULL_TREE; /* not simple */
+
+ return build2 ((enum tree_code)op.code, op.type, op0, op1);
+ }
}
return NULL_TREE;
@@ -5916,17 +5796,18 @@ maybe_fold_and_comparisons (tree type,
enum tree_code code1, tree op1a, tree op1b,
enum tree_code code2, tree op2a, tree op2b)
{
- if (tree t = and_comparisons_1 (type, code1, op1a, op1b, code2, op2a, op2b))
- return t;
-
- if (tree t = and_comparisons_1 (type, code2, op2a, op2b, code1, op1a, op1b))
- return t;
if (tree t = maybe_fold_comparisons_from_match_pd (type, BIT_AND_EXPR, code1,
op1a, op1b, code2, op2a,
op2b))
return t;
+ if (tree t = and_comparisons_1 (type, code1, op1a, op1b, code2, op2a, op2b))
+ return t;
+
+ if (tree t = and_comparisons_1 (type, code2, op2a, op2b, code1, op1a, op1b))
+ return t;
+
return NULL_TREE;
}
@@ -6391,15 +6272,15 @@ maybe_fold_or_comparisons (tree type,
enum tree_code code1, tree op1a, tree op1b,
enum tree_code code2, tree op2a, tree op2b)
{
- if (tree t = or_comparisons_1 (type, code1, op1a, op1b, code2, op2a, op2b))
+ if (tree t = maybe_fold_comparisons_from_match_pd (type, BIT_IOR_EXPR, code1,
+ op1a, op1b, code2, op2a,
+ op2b))
return t;
- if (tree t = or_comparisons_1 (type, code2, op2a, op2b, code1, op1a, op1b))
+ if (tree t = or_comparisons_1 (type, code1, op1a, op1b, code2, op2a, op2b))
return t;
- if (tree t = maybe_fold_comparisons_from_match_pd (type, BIT_IOR_EXPR, code1,
- op1a, op1b, code2, op2a,
- op2b))
+ if (tree t = or_comparisons_1 (type, code2, op2a, op2b, code1, op1a, op1b))
return t;
return NULL_TREE;
diff --git a/gcc/match.pd b/gcc/match.pd
index 2ca88000cad..ac80dd7dd15 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1958,6 +1958,72 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (eqne == NE_EXPR)
{ constant_boolean_node (true, type); }))))
+/* Convert (X == CST1) && (X OP2 CST2) to a known value
+ based on CST1 OP2 CST2. Similarly for (X != CST1). */
+
+(for code1 (eq ne)
+ (for code2 (eq ne lt gt le ge)
+ (simplify
+ (bit_and:c (code1@3 @0 INTEGER_CST@1) (code2@4 @0 INTEGER_CST@2))
+ (with
+ {
+ int cmp = tree_int_cst_compare (@1, @2);
+ bool val;
+ switch (code2)
+ {
+ case EQ_EXPR: val = (cmp == 0); break;
+ case NE_EXPR: val = (cmp != 0); break;
+ case LT_EXPR: val = (cmp < 0); break;
+ case GT_EXPR: val = (cmp > 0); break;
+ case LE_EXPR: val = (cmp <= 0); break;
+ case GE_EXPR: val = (cmp >= 0); break;
+ default: gcc_unreachable ();
+ }
+ }
+ (switch
+ (if (code1 == EQ_EXPR && val) @3)
+ (if (code1 == EQ_EXPR && !val) { constant_boolean_node (false, type); })
+ (if (code1 == NE_EXPR && !val) @4))))))
+
+/* Convert (X OP1 CST1) && (X OP2 CST2). */
+
+(for code1 (lt le gt ge)
+ (for code2 (lt le gt ge)
+ (simplify
+ (bit_and (code1:c@3 @0 INTEGER_CST@1) (code2:c@4 @0 INTEGER_CST@2))
+ (with
+ {
+ int cmp = tree_int_cst_compare (@1, @2);
+ }
+ (switch
+ /* Choose the more restrictive of two < or <= comparisons. */
+ (if ((code1 == LT_EXPR || code1 == LE_EXPR)
+ && (code2 == LT_EXPR || code2 == LE_EXPR))
+ (if ((cmp < 0) || (cmp == 0 && code1 == LT_EXPR))
+ @3
+ @4))
+ /* Likewise chose the more restrictive of two > or >= comparisons. */
+ (if ((code1 == GT_EXPR || code1 == GE_EXPR)
+ && (code2 == GT_EXPR || code2 == GE_EXPR))
+ (if ((cmp > 0) || (cmp == 0 && code1 == GT_EXPR))
+ @3
+ @4))
+ /* Check for singleton ranges. */
+ (if (cmp == 0
+ && ((code1 == LE_EXPR && code2 == GE_EXPR)
+ || (code1 == GE_EXPR && code2 == LE_EXPR)))
+ (eq @0 @1))
+ /* Check for disjoint ranges. */
+ (if (cmp <= 0
+ && (code1 == LT_EXPR || code1 == LE_EXPR)
+ && (code2 == GT_EXPR || code2 == GE_EXPR))
+ { constant_boolean_node (false, type); })
+ (if (cmp >= 0
+ && (code1 == GT_EXPR || code1 == GE_EXPR)
+ && (code2 == LT_EXPR || code2 == LE_EXPR))
+ { constant_boolean_node (false, type); })
+ )))))
+
/* We can't reassociate at all for saturating types. */
(if (!TYPE_SATURATING (type))
--
2.23.0
