public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tom de Vries <vries@codesourcery.com>
To: Zdenek Dvorak <rakdver@kam.mff.cuni.cz>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH PR45098,  7/10] Nowrap limits iterations
Date: Tue, 31 May 2011 09:07:00 -0000	[thread overview]
Message-ID: <4DE49E91.60502@codesourcery.com> (raw)
In-Reply-To: <20110530123851.GA29240@kam.mff.cuni.cz>

On 05/30/2011 02:38 PM, Zdenek Dvorak wrote:
> Hi,
> 
>>> The header block of the loop is bb 4, the latch block is bb 3:
>>> ...
>>> (gdb) p loop.header.index
>>> $4 = 4
>>> (gdb) p loop.latch.index
>>> $5 = 3
>>> ...
>>>
>>> The number of times the latch edge is executed, is 10.
>>>
>>> But loop->nb_iterations_upper_bound, or max_niter is 11:
> 
> this is a bit strange, it looks like the # of iterations estimation is setting
> nb_iterations_upper_bound too conservatively (or I gave nb_iterations_upper_bound
> a different semantics than I remember -- but both my memory and the comment in cfgloop.h
> suggest that nb_iterations_upper_bound >= nb_iterations, i.e., that it should be 10 in your
> example),
> 

The actual values of nb_iterations_upper_bound are determined by this code
fragment in record_estimate.

/* Records that AT_STMT is executed at most BOUND + 1 times in LOOP.  IS_EXIT
   is true if the loop is exited immediately after STMT, and this exit
   is taken at last when the STMT is executed BOUND + 1 times.
   REALISTIC is true if BOUND is expected to be close to the real number
   of iterations.  UPPER is true if we are sure the loop iterates at most
   BOUND times.  I_BOUND is an unsigned double_int upper estimate on BOUND.  */

static void
record_estimate (struct loop *loop, tree bound, double_int i_bound,
		 gimple at_stmt, bool is_exit, bool realistic, bool upper)
{
  ...

  /* Update the number of iteration estimates according to the bound.
     If at_stmt is an exit, then every statement in the loop is
     executed at most BOUND + 1 times.  If it is not an exit, then
     some of the statements before it could be executed BOUND + 2
     times, if an exit of LOOP is before stmt.  */
  exit = single_exit (loop);
  if (is_exit
      || (exit != NULL
	  && dominated_by_p (CDI_DOMINATORS,
			     exit->src, gimple_bb (at_stmt))))
    delta = double_int_one;
  else
    delta = double_int_two;
  i_bound = double_int_add (i_bound, delta);


As far as I can tell, what is current calculated in i_bound (and assigned to
nb_iterations_upper_bound), is the maximum amount of times any statement in the
loop is executed, where any includes exit tests. Differently put, the maximum
amount of times the loop header is executed.

This is confirmed by this comment in tree-vrp.c:

  /* Try to use estimated number of iterations for the loop to constrain the
     final value in the evolution.
     We are interested in the number of executions of the latch, while
     nb_iterations_upper_bound includes the last execution of the exit test.  */

I modified the patch to improved the comment.

Ok for trunk?

Thanks,
- Tom

Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c (revision 173734)
+++ gcc/tree-ssa-loop-ivopts.c (working copy)
@@ -4391,8 +4391,14 @@ may_eliminate_iv (struct ivopts_data *da
             {
               if (!estimated_loop_iterations (loop, true, &max_niter))
                 return false;
-              /* The loop bound is already adjusted by adding 1.  */
-              if (double_int_ucmp (max_niter, period_value) > 0)
+              /* (a) loop->nb_iterations_upper_bound (assigned to max_niter)
+                     includes the last execution of the exit test.
+                 (b) The number of distinct values of the cand is
+                     period_value + 1.
+                 So, the transformation is allowed if
+                 max_niter <= period_value + 1.  */
+              period_value = double_int_add (period_value, double_int_one);
+              if (double_int_ucmp (max_niter, period_value) > 0)
                 return false;
             }
           else

  reply	other threads:[~2011-05-31  7:53 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-17  7:23 [PATCH, PR45098] Tom de Vries
2011-05-17  7:25 ` [PATCH, PR45098, 1/10] Tom de Vries
2011-05-19 11:17   ` [PATCH PR45098, 1/10] Proc object-size fix Tom de Vries
2011-05-17  8:12 ` [PATCH, PR45098, 2/10] Tom de Vries
2011-05-18  9:39   ` Zdenek Dvorak
2011-05-17  8:30 ` [PATCH, PR45098, 3/10] Tom de Vries
2011-05-18 10:10   ` Zdenek Dvorak
2011-05-18 13:00     ` Tom de Vries
     [not found]       ` <20110518152457.GA13360@kam.mff.cuni.cz>
2011-05-18 19:30         ` Tom de Vries
2011-05-17  8:32 ` [PATCH, PR45098, 4/10] Tom de Vries
2011-05-18 17:46   ` [PATCH PR45098, 4/10] Iv init cost Tom de Vries
2011-05-18 22:59     ` Zdenek Dvorak
2011-05-25 14:20     ` Richard Sandiford
2011-05-26 12:24       ` Tom de Vries
2011-05-31 15:22         ` Richard Sandiford
2011-05-17  8:37 ` [PATCH, PR45098, 5/10] Tom de Vries
2011-05-18 17:48   ` [PATCH PR45098, 5/10] Bound cost Tom de Vries
2011-05-19  4:45     ` Zdenek Dvorak
2011-05-17  8:42 ` [PATCH, PR45098, 6/10] Tom de Vries
2011-05-18 17:48   ` [PATCH PR45098, 6/10] Bound cost - test cases Tom de Vries
2011-05-17  8:58 ` [PATCH, PR45098, 7/10] Tom de Vries
2011-05-18 17:52   ` [PATCH PR45098, 7/10] Nowrap limits iterations Tom de Vries
2011-05-19  4:45     ` Zdenek Dvorak
2011-05-20 12:22       ` Tom de Vries
2011-05-21 18:54         ` Zdenek Dvorak
2011-05-21 22:53           ` Tom de Vries
2011-05-28 17:58             ` Tom de Vries
2011-05-30 15:12               ` Zdenek Dvorak
2011-05-31  9:07                 ` Tom de Vries [this message]
2011-05-31  9:11                   ` Zdenek Dvorak
2011-06-11 10:13                     ` Tom de Vries
2011-06-12  1:17                       ` Zdenek Dvorak
2011-05-23 14:50           ` H.J. Lu
2011-05-17  9:03 ` [PATCH, PR45098, 8/10] Tom de Vries
2011-05-18 18:23   ` [PATCH PR45098, 8/10] Nowrap limits iterations - test cases Tom de Vries
2011-05-18 18:27   ` [PATCH PR45098, 9/10] Cheap shift-add Tom de Vries
2011-05-19  5:33     ` Zdenek Dvorak
2011-05-20 11:32       ` Tom de Vries
2011-05-20 20:09         ` Zdenek Dvorak
2011-05-21 15:05         ` Eric Botcazou
2011-05-22 19:33           ` Tom de Vries
2011-05-22 20:22             ` Richard Guenther
2011-05-22 21:11             ` Eric Botcazou
2011-05-17 10:03 ` [PATCH, PR45098, 9/10] Tom de Vries
2011-05-17 10:30 ` [PATCH, PR45098, 10/10] Tom de Vries
2011-05-18 18:30   ` [PATCH PR45098, 10/10] Cheap shift-add - test case Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DE49E91.60502@codesourcery.com \
    --to=vries@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rakdver@kam.mff.cuni.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).