From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111281 invoked by alias); 20 Jan 2020 13:03:02 -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 111271 invoked by uid 89); 20 Jan 2020 13:03:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy= X-HELO: gate.crashing.org Received: from gate.crashing.org (HELO gate.crashing.org) (63.228.1.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Jan 2020 13:02:51 +0000 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 00KD2nlM027731; Mon, 20 Jan 2020 07:02:49 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 00KD2n6S027730; Mon, 20 Jan 2020 07:02:49 -0600 Date: Mon, 20 Jan 2020 13:12:00 -0000 From: Segher Boessenkool To: "Kewen.Lin" Cc: GCC Patches , Bill Schmidt , "bin.cheng" , Richard Guenther Subject: Re: [PATCH 1/4 GCC11] Add middle-end unroll factor estimation Message-ID: <20200120130249.GW3191@gate.crashing.org> References: <131a3294-1951-3678-453b-085744366af6@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <131a3294-1951-3678-453b-085744366af6@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg01211.txt.bz2 Hi! On Thu, Jan 16, 2020 at 05:39:40PM +0800, Kewen.Lin wrote: > --- a/gcc/cfgloop.h > +++ b/gcc/cfgloop.h > @@ -232,6 +232,9 @@ public: > Other values means unroll with the given unrolling factor. */ > unsigned short unroll; > > + /* Like unroll field above, but it's estimated in middle-end. */ > + unsigned short estimated_uf; Please use full words? "estimated_unroll" perhaps? (Similar for other new names). > +/* Implement targetm.loop_unroll_adjust_tree, strictly refers to > + targetm.loop_unroll_adjust. */ > + > +static unsigned > +rs6000_loop_unroll_adjust_tree (unsigned nunroll, struct loop *loop) > +{ > + /* For now loop_unroll_adjust is simple, just invoke directly. */ > + return rs6000_loop_unroll_adjust (nunroll, loop); > +} Since the two hooks have the same arguments as well, it should really just be one hook, and an implementation can check whether current_pass->type == RTL_PASS if it needs to do something special for RTL, etc.? Or it can use some more appropriate condition -- the point is you need no extra hook. > + /* Check number of iterations is constant. */ > + if ((niter_desc->may_be_zero && !integer_zerop (niter_desc->may_be_zero)) > + || !tree_fits_uhwi_p (niter_desc->niter)) > + return false; Check, and do what? It's easier to read if you say. > + /* Check for an explicit unrolling factor. */ > + if (loop->unroll > 0 && loop->unroll < USHRT_MAX) > + { > + /* It should have been peeled instead. */ > + if (const_niter == 0 || (unsigned) loop->unroll > const_niter - 1) > + loop->estimated_uf = 1; > + else > + loop->estimated_uf = loop->unroll; > + return true; > + } "If loop->unroll is set, use that as loop->estimated_unroll"? Segher