From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 50112 invoked by alias); 26 Jul 2017 09:08:45 -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 50101 invoked by uid 89); 26 Jul 2017 09:08:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=realise, struggled X-HELO: mail-wm0-f45.google.com Received: from mail-wm0-f45.google.com (HELO mail-wm0-f45.google.com) (74.125.82.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Jul 2017 09:08:42 +0000 Received: by mail-wm0-f45.google.com with SMTP id c184so75787978wmd.0 for ; Wed, 26 Jul 2017 02:08:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version; bh=lQNPBT4aHmMfQeJu1UDLJcFTupWdoggPCQKyjCkzW4A=; b=K+15hRmFOn86mwOrjOBbg/u8L5J048VrrMob60A8uTR5f6h9Ts4XKtqBLLuWn4sfb4 5a2xiLJuXCFRi3rSTP1bb3wsaBFYD328A7BCYUzsZUta/5ES4W2gvV7L85WyNPW97B5v Nh5KFJmtgtZjHfth0K6CEjBLuYsSA3CEYELOKGNevlWUqS0e3bZK0P/U6KOJ2ErhVt+n 0RHKO4IUUOpEsnjGx+MCBTeK/n+pAOn1EsdCy1VdUTdHgia+W3FKt46493TNYJO+n/7P V5cKaGntwNvmK5L0ekvZfrOP5xh1sP0RD6/2MYGloyLaWpSunYw6i1HVGS7vNHsx16if YsdQ== X-Gm-Message-State: AIVw110Lr7PKD2O5GlKLjZxt4Xrab7mU8uFi0CFLRgmCZh2omTyn/n+g o1Y8AuUfcEEWN5FBCvwd6w== X-Received: by 10.28.215.206 with SMTP id o197mr219640wmg.40.1501060120514; Wed, 26 Jul 2017 02:08:40 -0700 (PDT) Received: from localhost ([2.26.27.176]) by smtp.gmail.com with ESMTPSA id 16sm8757286wrx.26.2017.07.26.02.08.38 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 26 Jul 2017 02:08:39 -0700 (PDT) From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener ,Marc Glisse , "Bin.Cheng" , gcc-patches List , richard.sandiford@linaro.org Cc: Marc Glisse , "Bin.Cheng" , gcc-patches List Subject: Re: [PATCH GCC][1/2]Feed bound computation to folder in loop split References: Date: Wed, 26 Jul 2017 09:08:00 -0000 In-Reply-To: (Richard Biener's message of "Wed, 26 Jul 2017 09:48:31 +0200") Message-ID: <87y3rbva3y.fsf@linaro.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2017-07/txt/msg01643.txt.bz2 Richard Biener writes: > On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse wrote: >> On Tue, 25 Jul 2017, Richard Biener wrote: >> >>>> I think we need Richard to say what the intent is for the valueization >>>> function. It is used both to stop looking at defining stmt if the return >>>> is >>>> NULL, and to replace/optimize one SSA_NAME with another, but currently it >>>> seems hard to prevent looking at the defining statement without >>>> preventing >>>> from looking at the SSA_NAME at all. >>> >>> >>> Yeah, this semantic overloading is an issue. For gimple_build we have >>> nothing >>> to "valueize" but we only use it to tell genmatch that it may not look at >>> the >>> SSA_NAME_DEF_STMT. >>> >>>> I guess we'll need a fix in genmatch... >>> >>> >>> I'll have a look tomorrow. >> >> >> My impression yesterday was that we could replace the current do_valueize >> wrapper by 2 wrappers (without touching the valueize callbacks): >> - may_check_def_stmt, which returns a bool corresponding to the current >> do_valueize != NULL_TREE >> - maybe_valueize, which tries to valueize, but if it gets a NULL_TREE, it >> returns its argument unchanged. >> >> Not very confident about it though. > > Note I've been there in the past (twice I think) but always ran into existing > latent issues. So hopefully we've resolved those, I'm testing the following > simplified variant of what I had back in time. > > It'll produce > > switch (TREE_CODE (op0)) > { > case SSA_NAME: > if (gimple *def_stmt = get_def (valueize, op0)) > { > if (gassign *def = dyn_cast (def_stmt)) > switch (gimple_assign_rhs_code (def)) > { > case MINUS_EXPR: > { > tree o20 = gimple_assign_rhs1 (def); > o20 = do_valueize (valueize, o20); > tree o21 = gimple_assign_rhs2 (def); > o21 = do_valueize (valueize, o21); > if (op1 == o21 || (operand_equal_p (op1, o21, 0) && > types_match (op1, o21))) > { > > which also indents less which is nice. > > Bootstrap/regtest running on x86_64-unknown-linux-gnu. > > Richard. > > 2017-07-26 Richard Biener > > * gimple-match-head.c (do_valueize): Return OP if valueize > returns NULL_TREE. > (get_def): New helper to get at the def stmt of a SSA name > if valueize allows. > * genmatch.c (dt_node::gen_kids_1): Use get_def instead of > do_valueize to get at the def stmt. > (dt_operand::gen_gimple_expr): Simplify do_valueize calls. > > >> -- >> Marc Glisse > > 2017-07-26 Richard Biener > > * gimple-match-head.c (do_valueize): Return OP if valueize > returns NULL_TREE. > (get_def): New helper to get at the def stmt of a SSA name > if valueize allows. > * genmatch.c (dt_node::gen_kids_1): Use get_def instead of > do_valueize to get at the def stmt. > (dt_operand::gen_gimple_expr): Simplify do_valueize calls. > > Index: gcc/gimple-match-head.c > =================================================================== > --- gcc/gimple-match-head.c (revision 250518) > +++ gcc/gimple-match-head.c (working copy) > @@ -779,10 +779,25 @@ inline tree > do_valueize (tree (*valueize)(tree), tree op) > { > if (valueize && TREE_CODE (op) == SSA_NAME) > - return valueize (op); > + { > + tree tem = valueize (op); > + if (tem) > + return tem; > + } > return op; > } > > +/* Helper for the autogenerated code, get at the definition of NAME when > + VALUEIZE allows that. */ > + > +inline gimple * > +get_def (tree (*valueize)(tree), tree name) > +{ > + if (valueize && ! valueize (name)) > + return NULL; > + return SSA_NAME_DEF_STMT (name); > +} I realise this is preexisting, but why do we ignore the value returned by valueize, even if it's different from NAME? I struggled over that with the old code when I hit the same problem with some SVE patches (for which, thanks for the fix). A comment might be good. Thanks, Richard