From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 50849 invoked by alias); 11 Sep 2019 13:57:12 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 50840 invoked by uid 89); 11 Sep 2019 13:57:11 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 11 Sep 2019 13:57:09 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id E4E5069886; Wed, 11 Sep 2019 13:57:06 +0000 (UTC) Subject: Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd. From: =?UTF-8?Q?Martin_Li=c5=a1ka?= To: Richard Biener Cc: gcc-patches@gcc.gnu.org, Li Jia He , Andrew Pinski , Jeff Law , Segher Boessenkool , wschmidt@linux.ibm.com, Martin Liska References: <1561615913-22109-1-git-send-email-helijia@linux.ibm.com> <6fb28248-5134-cec5-5045-45253e4d2eb0@redhat.com> <6d333ccf-9905-e929-c2dc-fc611ff929f1@linux.ibm.com> <845bc280-7bd6-509b-3830-4ebde50f1b20@linux.ibm.com> <4efa66d4-c04c-e5a6-18ff-2f4310d7f64d@suse.cz> <3b78e414-ff03-daa8-448f-4eaf766a2635@suse.cz> <8e67d843-c37b-06cd-52ea-943c97d58a87@suse.cz> <09b56054-cea4-a61d-f3b6-5d3ae1a92d75@suse.cz> <55116893-fd3b-1da4-ece2-952d97a49ce7@suse.cz> Message-ID: <9b2bcc34-6925-7623-9c46-f4704ebeab36@suse.cz> Date: Wed, 11 Sep 2019 13:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <55116893-fd3b-1da4-ece2-952d97a49ce7@suse.cz> Content-Type: multipart/mixed; boundary="------------2BEA1400EB4893D9E1E2DE46" X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00750.txt.bz2 This is a multi-part message in MIME format. --------------2BEA1400EB4893D9E1E2DE46 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-length: 3457 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 (kids[i])) >> + if ((s2 = dyn_cast (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 >>> >> > --------------2BEA1400EB4893D9E1E2DE46 Content-Type: text/x-patch; name="0003-Rewrite-part-of-and_comparisons_1-into-match.pd.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0003-Rewrite-part-of-and_comparisons_1-into-match.pd.patch" Content-length: 10879 >From f4e6cf9aec14111a35b1c8d641f83ec355d8c7e0 Mon Sep 17 00:00:00 2001 From: Martin Liska 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 * 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 (kids[i])) + if ((s2 = dyn_cast (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 --------------2BEA1400EB4893D9E1E2DE46--