From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8925 invoked by alias); 11 Sep 2019 13:19:05 -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 8910 invoked by uid 89); 11 Sep 2019 13:19:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=H*i:sk:nycvar. 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:19:03 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 2885BB755; Wed, 11 Sep 2019 13:19:01 +0000 (UTC) Subject: Re: [PATCH 3/5] Rewrite part of and_comparisons_1 into match.pd. 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> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <55116893-fd3b-1da4-ece2-952d97a49ce7@suse.cz> Date: Wed, 11 Sep 2019 13:19: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: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00742.txt.bz2 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 >> >