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 +++
3 files changed, 93
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;
- swi
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
