From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10601 invoked by alias); 23 May 2014 17:48:06 -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 10583 invoked by uid 89); 23 May 2014 17:48:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 23 May 2014 17:48:04 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 4632B541A24; Fri, 23 May 2014 19:48:00 +0200 (CEST) Date: Fri, 23 May 2014 17:48:00 -0000 From: Jan Hubicka To: Richard Biener Cc: Jan Hubicka , Dehao Chen , GCC Patches , David Li Subject: Re: [PATCH] Disable unroll loop that has header count less than iteration count. Message-ID: <20140523174759.GD30708@kam.mff.cuni.cz> References: <20140523172604.GB30708@kam.mff.cuni.cz> <8ecc97fc-bcc3-45e4-a415-97c5fd85ebe6@email.android.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8ecc97fc-bcc3-45e4-a415-97c5fd85ebe6@email.android.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-05/txt/msg02035.txt.bz2 > > > > if (expected_loop_iterations_reliable_p (loop)) > > niters = expected_loop_iterations (loop); > > But why not simply never return bogus values from expected-loop-iterations? Hmm, actually we do have: /* If we have a measured profile, use it to estimate the number of iterations. */ if (loop->header->count != 0) { gcov_type nit = expected_loop_iterations_unbounded (loop) + 1; bound = gcov_type_to_wide_int (nit); record_niter_bound (loop, bound, true, false); } and get_estimated_loop_iterations that has the behaviour intended. Forgot about this. So probably we want to revisit remaining uses of expected_loop_iterations and use get_estimated_loop_iterations (most of compiler actually does that). I did some of these changes in past, so there are not many left. I would move the logic setting the actual estimate based on profile from estimate_numbers_of_iterations_loop into tree-profile pass (i.e. do it once at profile load time and maintain it all the way through compilation them, such as in inlining). AtoFDO can do its own analysis: I suppose loop count is known to autoFDO when it can find source line that is always executed in the loop and source line that is known to have same count as header. This may be implementable as a separate analysis rather than having heuristic based on overall sanity of the profile around the loop. Honza > We probably want a flag in the .gcda file on whether it was from auto-fdo and only not trust profiles from those. > > Richard. > > >We probably also want to store this information into loop structure > >rather than computing > >it all the time from profile, since the profile may get inaccurate and > >we already know > >how to maintain upper bounds on numbers of iterations, so it should be > >easy to implement. > > > >This would allow us to preserve info like > >for (i=0 ;i < __bulitin_expect (n,10); i++) > > > >that would be nice feature to have. > > > >Honza > > > >> you run into? > >> > >> /* Returns expected number of LOOP iterations. The returned value is > >bounded > >> by REG_BR_PROB_BASE. */ > >> > >> unsigned > >> expected_loop_iterations (const struct loop *loop) > >> { > >> gcov_type expected = expected_loop_iterations_unbounded (loop); > >> return (expected > REG_BR_PROB_BASE ? REG_BR_PROB_BASE : expected); > >> } > >> > >> I miss a testcase as well. > >> > >> Richard. > >> > >> > Thanks, > >> > Dehao > >> > > >> > gcc/ChangeLog: > >> > 2014-05-21 Dehao Chen > >> > > >> > * cfgloop.h (expected_loop_iterations_reliable_p): New > >func. > >> > * cfgloopanal.c (expected_loop_iterations_reliable_p): > >Likewise. > >> > * loop-unroll.c (decide_unroll_runtime_iterations): Disable > >unroll > >> > loop that has unreliable iteration counts. > >> > > >> > Index: gcc/cfgloop.h > >> > =================================================================== > >> > --- gcc/cfgloop.h (revision 210717) > >> > +++ gcc/cfgloop.h (working copy) > >> > @@ -307,8 +307,8 @@ extern bool just_once_each_iteration_p (const > >stru > >> > gcov_type expected_loop_iterations_unbounded (const struct loop > >*); > >> > extern unsigned expected_loop_iterations (const struct loop *); > >> > extern rtx doloop_condition_get (rtx); > >> > +extern bool expected_loop_iterations_reliable_p (const struct loop > >*); > >> > > >> > - > >> > /* Loop manipulation. */ > >> > extern bool can_duplicate_loop_p (const struct loop *loop); > >> > > >> > Index: gcc/cfgloopanal.c > >> > =================================================================== > >> > --- gcc/cfgloopanal.c (revision 210717) > >> > +++ gcc/cfgloopanal.c (working copy) > >> > @@ -285,6 +285,15 @@ expected_loop_iterations (const struct loop > >*loop) > >> > return (expected > REG_BR_PROB_BASE ? REG_BR_PROB_BASE : > >expected); > >> > } > >> > > >> > +/* Returns true if the loop header's profile count is smaller than > >expected > >> > + loop iteration. */ > >> > + > >> > +bool > >> > +expected_loop_iterations_reliable_p (const struct loop *loop) > >> > +{ > >> > + return expected_loop_iterations (loop) < loop->header->count; > >> > +} > >> > + > >> > /* Returns the maximum level of nesting of subloops of LOOP. */ > >> > > >> > unsigned > >> > Index: gcc/loop-unroll.c > >> > =================================================================== > >> > --- gcc/loop-unroll.c (revision 210717) > >> > +++ gcc/loop-unroll.c (working copy) > >> > @@ -988,6 +988,15 @@ decide_unroll_runtime_iterations (struct loop > >*loo > >> > return; > >> > } > >> > > >> > + if (profile_status_for_fn (cfun) == PROFILE_READ > >> > + && expected_loop_iterations_reliable_p (loop)) > >> > + { > >> > + if (dump_file) > >> > + fprintf (dump_file, ";; Not unrolling loop, loop iteration " > >> > + "not reliable."); > >> > + return; > >> > + } > >> > + > >> > /* Check whether the loop rolls. */ > >> > if ((get_estimated_loop_iterations (loop, &iterations) > >> > || get_max_loop_iterations (loop, &iterations)) >