Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
On 9/16/19 5:04 AM, Richard Biener wrote: On Wed, 11 Sep 2019, Martin Liška wrote: On 9/11/19 3:19 PM, Martin Liška wrote: On 9/11/19 2:43 PM, Richard Biener wrote: Any particular reason you needed to swap the calls in maybe_fold_and/or_comparisons? You didn't answer this, besides that the patch is OK. Ah, sorry. No, there's not any particular reason. My motivation was that I moved the patterns from the beginning of and_comparisons_1 to match.pd. So that I wanted to begin with the maybe_fold_comparisons_from_match_pd. I'll put it back to the original order. Martin Thanks, Richard.
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
On Wed, 11 Sep 2019, Martin Liška wrote: > On 9/11/19 3:19 PM, Martin Liška wrote: > > On 9/11/19 2:43 PM, Richard Biener wrote: > >> Any particular reason you needed to swap the calls in > >> maybe_fold_and/or_comparisons? You didn't answer this, besides that the patch is OK. Thanks, Richard.
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
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 >>> >> > >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 ++
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
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. > > + 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. Martin > > Otherwise OK. > > Thanks, > Richard. > > > > >> Thanks, >> Martin >> >
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
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. + /* 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. + 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. 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? Otherwise OK. Thanks, Richard. > Thanks, > Martin > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 247165 (AG München)
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
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? Thanks, Martin >From e21404f55f51701d25a6d859b892b219d2041e02 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 (same_bool_result_p): Handle SSA_NAMEs created in maybe_fold_comparisons_from_match_pd. (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| 4 +- gcc/gimple-fold.c | 174 +- gcc/match.pd | 68 ++ 3 files changed, 105 insertions(+), 141 deletions(-) diff --git a/gcc/genmatch.c b/gcc/genmatch.c index 2e7bf27eeda..b7194448a0f 100644 --- 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) { warning_at (s->match->location, "duplicate pattern"); warning_at (s2->s->match->location, "previous pattern defined here"); diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index aa2f94ace28..58235d613d3 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -5350,6 +5350,19 @@ same_bool_result_p (const_tree op1, const_tree op2) if (operand_equal_p (op1, op2, 0)) return true; + /* 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; +} + /* Check the cases where at least one of the operands is a comparison. These are a bit smarter than operand_equal_p in that they apply some identifies on SSA_NAMEs. */ @@ -5620,136 +5633,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 &
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
On 9/10/19 1:19 PM, Marc Glisse wrote: > (some quick comments, I didn't check in details) Thanks for them. > > + (and:c (code1 @0 INTEGER_CST@1) (code2 @0 INTEGER_CST@2)) > [...] > + (if (code1 == NE_EXPR && !val) (code2 @0 @2 > > How about > > (and:c (code1 @0 INTEGER_CST@1) (code2@3 @0 INTEGER_CST@2)) > [...] > (if (code1 == NE_EXPR && !val) @3))) > > instead? This is also a way to check how well the generator handles this, > when @3 may not really exist (was in a gimple_cond). I like the changes. I applied them to all patterns in the patch set. > > (and:c (eq@3 @0 INTEGER_CST@1) (code2 @0 INTEGER_CST@2)) > > could be simplified to > > (and @3 (code2 @1 @2)) > > always (a trivial transform) and let the rest of the machinery fold code2 on > constants and then and with a constant. This way you would only need to > handle code1==NE_EXPR in the complicated transform, it might be slightly more > readable. (I am not saying we absolutely have to do it this way, it is just a > suggestion) That would be possible, I can experiment with that after the patch set is applied. The current patch is more or less direct transformation of what we have in gimple-fold.c. > > + (and (code1:c @0 INTEGER_CST@1) (code2:c @0 INTEGER_CST@2)) > > Don't we canonicalize 3 < X to X > 3? That would make the :c unnecessary. > Actually I don't remember how :c works on non-commutative operations (there > was also :C I think). Dunno here, Richi? > > + /* Chose the more restrictive of two < or <= comparisons. */ > > Choose? Yep. Thank you, Martin >
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
On 9/10/19 10:52 AM, Bernhard Reutner-Fischer wrote: > On 9 September 2019 15:41:05 CEST, "Martin Liška" wrote: >> On 9/9/19 2:24 PM, Martin Liška wrote: >>> Hi. >>> >>> The patch is about transition of and_comparisons_1 matching >>> into match.pd. >>> >>> Patch can bootstrap on x86_64-linux-gnu and survives regression >> tests. >>> >>> Ready to be installed? >>> Thanks, >>> Martin >>> >> >> Updated version (as mentioned in part 1). >> > > +&& TREE_CODE_CLASS ((tree_code)op.code) == tcc_comparison) > > COMPARISON_CLASS_P > > thanks, > Thanks for the comment. Martin
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
(some quick comments, I didn't check in details) +(and:c (code1 @0 INTEGER_CST@1) (code2 @0 INTEGER_CST@2)) [...] + (if (code1 == NE_EXPR && !val) (code2 @0 @2 How about (and:c (code1 @0 INTEGER_CST@1) (code2@3 @0 INTEGER_CST@2)) [...] (if (code1 == NE_EXPR && !val) @3))) instead? This is also a way to check how well the generator handles this, when @3 may not really exist (was in a gimple_cond). (and:c (eq@3 @0 INTEGER_CST@1) (code2 @0 INTEGER_CST@2)) could be simplified to (and @3 (code2 @1 @2)) always (a trivial transform) and let the rest of the machinery fold code2 on constants and then and with a constant. This way you would only need to handle code1==NE_EXPR in the complicated transform, it might be slightly more readable. (I am not saying we absolutely have to do it this way, it is just a suggestion) + (and (code1:c @0 INTEGER_CST@1) (code2:c @0 INTEGER_CST@2)) Don't we canonicalize 3 < X to X > 3? That would make the :c unnecessary. Actually I don't remember how :c works on non-commutative operations (there was also :C I think). + /* Chose the more restrictive of two < or <= comparisons. */ Choose? -- Marc Glisse
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
On 9 September 2019 15:41:05 CEST, "Martin Liška" wrote: >On 9/9/19 2:24 PM, Martin Liška wrote: >> Hi. >> >> The patch is about transition of and_comparisons_1 matching >> into match.pd. >> >> Patch can bootstrap on x86_64-linux-gnu and survives regression >tests. >> >> Ready to be installed? >> Thanks, >> Martin >> > >Updated version (as mentioned in part 1). > + && TREE_CODE_CLASS ((tree_code)op.code) == tcc_comparison) COMPARISON_CLASS_P thanks,
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
On 9/9/19 3:41 PM, Martin Liška wrote: On 9/9/19 2:24 PM, Martin Liška wrote: Hi. The patch is about transition of and_comparisons_1 matching into match.pd. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin Updated version (as mentioned in part 1). Martin And there's updated part 3 where I properly handle the TREE_CODE_CLASS ((tree_code)op.code) == tcc_comparison case. Martin >From 5e495f3eeeda3a2a850a3d61fbfd084b4a41328a 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 (same_bool_result_p): Handle SSA_NAMEs created in maybe_fold_comparisons_from_match_pd. (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| 4 +- gcc/gimple-fold.c | 174 +- gcc/match.pd | 68 ++ 3 files changed, 105 insertions(+), 141 deletions(-) diff --git a/gcc/genmatch.c b/gcc/genmatch.c index 2e7bf27eeda..b7194448a0f 100644 --- 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) { warning_at (s->match->location, "duplicate pattern"); warning_at (s2->s->match->location, "previous pattern defined here"); diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index fcdcb087ec4..ae7a363710c 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -5350,6 +5350,19 @@ same_bool_result_p (const_tree op1, const_tree op2) if (operand_equal_p (op1, op2, 0)) return true; + /* 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; +} + /* Check the cases where at least one of the operands is a comparison. These are a bit smarter than operand_equal_p in that they apply some identifies on SSA_NAMEs. */ @@ -5620,136 +5633,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; -
Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
On 9/9/19 2:24 PM, Martin Liška wrote: > Hi. > > The patch is about transition of and_comparisons_1 matching > into match.pd. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > Updated version (as mentioned in part 1). Martin >From a4127ffe3d55c67dc8ef78d62a27277807740995 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): Ignore warning for the same location. * gimple-fold.c (same_bool_result_p): Handle newly created SSA_NAMEs ar arguments. (and_comparisons_1): Add new argument gimple_stmt_iterator. (and_var_with_comparison): Likewise. (and_var_with_comparison_1): Likewise. (or_comparisons_1): Likewise. (or_var_with_comparison): Likewise. (or_var_with_comparison_1): Likewise. (maybe_fold_comparisons_from_match_pd): Handle creation of temporary SSA_NAMEs. Add new argument gimple_stmt_iterator. (maybe_fold_and_comparisons): Likewise. (maybe_fold_or_comparisons): Likewise. * gimple-fold.h (maybe_fold_and_comparisons): Likewise. (maybe_fold_or_comparisons): Likewise. * match.pd: Add rules for (X OP1 CST1) && (X OP2 CST2). * tree-if-conv.c (fold_or_predicates): Do not pass gimple_stmt_iterator. * tree-ssa-ifcombine.c (ifcombine_ifandif): Pass gimple_stmt_iterator. * tree-ssa-reassoc.c (eliminate_redundant_comparison): Do not pass gimple_stmt_iterator. (optimize_vec_cond_expr): Likewise. --- gcc/genmatch.c | 4 +- gcc/gimple-fold.c| 266 ++- gcc/gimple-fold.h| 6 +- gcc/match.pd | 68 ++ gcc/tree-if-conv.c | 2 +- gcc/tree-ssa-ifcombine.c | 5 +- gcc/tree-ssa-reassoc.c | 11 +- 7 files changed, 177 insertions(+), 185 deletions(-) diff --git a/gcc/genmatch.c b/gcc/genmatch.c index 2e7bf27eeda..b7194448a0f 100644 --- 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) { warning_at (s->match->location, "duplicate pattern"); warning_at (s2->s->match->location, "previous pattern defined here"); diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 50cb3bf7e32..d046603fd6f 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -5350,6 +5350,19 @@ same_bool_result_p (const_tree op1, const_tree op2) if (operand_equal_p (op1, op2, 0)) return true; + /* 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; +} + /* Check the cases where at least one of the operands is a comparison. These are a bit smarter than operand_equal_p in that they apply some identifies on SSA_NAMEs. */ @@ -5372,22 +5385,28 @@ same_bool_result_p (const_tree op1, const_tree op2) static tree and_comparisons_1 (enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree and_var_with_comparison (tree var, bool invert, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree and_var_with_comparison_1 (gimple *stmt, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree or_comparisons_1 (enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree or_var_with_comparison (tree var, bool invert, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree or_var_with_comparison_1 (gimple *stmt, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); /* Helper function for and_comparisons_1: try to simplify the AND of the ssa variable VAR with the comparison specified by (OP2A CODE2 OP2B). @@ -
[PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd.
Hi. The patch is about transition of and_comparisons_1 matching into match.pd. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin >From 15045058d6caf84734ea949a297b6e31d9a8647c 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): Ignore warning for the same location. * gimple-fold.c (same_bool_result_p): Handle newly created SSA_NAMEs ar arguments. (and_comparisons_1): Add new argument gimple_stmt_iterator. (and_var_with_comparison): Likewise. (and_var_with_comparison_1): Likewise. (or_comparisons_1): Likewise. (or_var_with_comparison): Likewise. (or_var_with_comparison_1): Likewise. (maybe_fold_comparisons_from_match_pd): Handle creation of temporary SSA_NAMEs. Add new argument gimple_stmt_iterator. (maybe_fold_and_comparisons): Likewise. (maybe_fold_or_comparisons): Likewise. * gimple-fold.h (maybe_fold_and_comparisons): Likewise. (maybe_fold_or_comparisons): Likewise. * match.pd: Add rules for (X OP1 CST1) && (X OP2 CST2). * tree-if-conv.c (fold_or_predicates): Do not pass gimple_stmt_iterator. * tree-ssa-ifcombine.c (ifcombine_ifandif): Pass gimple_stmt_iterator. * tree-ssa-reassoc.c (eliminate_redundant_comparison): Do not pass gimple_stmt_iterator. (optimize_vec_cond_expr): Likewise. --- gcc/genmatch.c | 4 +- gcc/gimple-fold.c| 261 +-- gcc/gimple-fold.h| 6 +- gcc/match.pd | 68 ++ gcc/tree-if-conv.c | 2 +- gcc/tree-ssa-ifcombine.c | 5 +- gcc/tree-ssa-reassoc.c | 11 +- 7 files changed, 172 insertions(+), 185 deletions(-) diff --git a/gcc/genmatch.c b/gcc/genmatch.c index 2e7bf27eeda..b7194448a0f 100644 --- 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) { warning_at (s->match->location, "duplicate pattern"); warning_at (s2->s->match->location, "previous pattern defined here"); diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 8a9eca13b87..f9971c004b7 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -5350,6 +5350,19 @@ same_bool_result_p (const_tree op1, const_tree op2) if (operand_equal_p (op1, op2, 0)) return true; + /* 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; +} + /* Check the cases where at least one of the operands is a comparison. These are a bit smarter than operand_equal_p in that they apply some identifies on SSA_NAMEs. */ @@ -5372,22 +5385,28 @@ same_bool_result_p (const_tree op1, const_tree op2) static tree and_comparisons_1 (enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree and_var_with_comparison (tree var, bool invert, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree and_var_with_comparison_1 (gimple *stmt, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree or_comparisons_1 (enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree or_var_with_comparison (tree var, bool invert, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); static tree or_var_with_comparison_1 (gimple *stmt, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + gimple_stmt_iterator *gsi); /* Helper function for and_comparisons_1: try to simplify the AND of the ssa variable VAR with the comparison specified by (OP2A CODE2 OP2B). @@ -5396,7 +5415,8 @@ or_var_with_comparison_1 (gimple *stmt, static tree and_var_with_comparison (tree var, bool invert, - enum tree_c