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.
>
> + 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
>>
>
