public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
To: Richard Biener <rguenther@suse.de>,Jan Hubicka <hubicka@ucw.cz>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: Do not give realistic estimates for loop with array accesses
Date: Wed, 30 Mar 2016 18:52:00 -0000	[thread overview]
Message-ID: <64947530-A439-42B4-826F-DA52AF33686D@gmail.com> (raw)
In-Reply-To: <alpine.LSU.2.11.1603301436071.13384@t29.fhfr.qr>

On March 30, 2016 2:36:14 PM GMT+02:00, Richard Biener <rguenther@suse.de> wrote:
>On Wed, 30 Mar 2016, Jan Hubicka wrote:
>
>> > 
>> > You are only changing one place in this file.
>> 
>> You are right. I am attaching the updated patch which I am re-testing
>now.
>> > 
>> > The vectorizer already checks this (albeit indirectly):
>> > 
>> >   HOST_WIDE_INT max_niter
>> >     = max_stmt_executions_int (LOOP_VINFO_LOOP (loop_vinfo));
>> >   if ((LOOP_VINFO_NITERS_KNOWN_P (loop_vinfo)
>> >        && (LOOP_VINFO_INT_NITERS (loop_vinfo) <
>vectorization_factor))
>> >       || (max_niter != -1
>> >           && (unsigned HOST_WIDE_INT) max_niter <
>vectorization_factor))
>> >     {
>> >       if (dump_enabled_p ())
>> >         dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>> >                          "not vectorized: iteration count smaller
>than "
>> >                          "vectorization factor.\n");
>> >       return false;
>> >     }
>> 
>> Yes, but one tests only vectorization_factor and other
>min_profitable_estimate
>> which probably should be greater than vectorization_factor.
>> 
>> The check above should therefore become redundant.  My reading of the
>code is
>> that min_profiltable_estimate is computed after the check above, so
>it is
>> probably an useful shortcut and the message is also bit more
>informative.
>> I updated the later test to use max_niter variable once it is
>computed.
>> 
>> OK with those changes assuming testing passes?
>
>Ok.

Maybe s/overlow/overflow/g while at it..

TIA,
>
>Richard.
>
>> Honza
>> 
>> 	* tree-ssa-loop-niter.c (idx_infer_loop_bounds): We can't get
>realistic
>> 	estimates here.
>> 	* tree-ssa-loop-unswitch.c (tree_unswitch_single_loop): Use also
>> 	max_loop_iterations_int.
>> 	(tree_unswitch_outer_loop): Likewise.
>> 	* tree-ssa-loop-ivopts.c (avg_loop_niter): Likewise.
>> 	* tree-vect-loop.c (vect_analyze_loop_2): Likewise.
>> Index: tree-ssa-loop-ivopts.c
>> ===================================================================
>> --- tree-ssa-loop-ivopts.c	(revision 234516)
>> +++ tree-ssa-loop-ivopts.c	(working copy)
>> @@ -121,7 +121,11 @@ avg_loop_niter (struct loop *loop)
>>  {
>>    HOST_WIDE_INT niter = estimated_stmt_executions_int (loop);
>>    if (niter == -1)
>> -    return AVG_LOOP_NITER (loop);
>> +    {
>> +      niter = max_stmt_executions_int (loop);
>> +      if (niter == -1 || niter > AVG_LOOP_NITER (loop))
>> +        return AVG_LOOP_NITER (loop);
>> +    }
>>  
>>    return niter;
>>  }
>> Index: tree-ssa-loop-niter.c
>> ===================================================================
>> --- tree-ssa-loop-niter.c	(revision 234516)
>> +++ tree-ssa-loop-niter.c	(working copy)
>> @@ -3115,7 +3115,6 @@ idx_infer_loop_bounds (tree base, tree *
>>    tree low, high, type, next;
>>    bool sign, upper = true, at_end = false;
>>    struct loop *loop = data->loop;
>> -  bool reliable = true;
>>  
>>    if (TREE_CODE (base) != ARRAY_REF)
>>      return true;
>> @@ -3187,14 +3186,14 @@ idx_infer_loop_bounds (tree base, tree *
>>        && tree_int_cst_compare (next, high) <= 0)
>>      return true;
>>  
>> -  /* If access is not executed on every iteration, we must ensure
>that overlow may
>> -     not make the access valid later.  */
>> +  /* If access is not executed on every iteration, we must ensure
>that overlow
>> +     may not make the access valid later.  */
>>    if (!dominated_by_p (CDI_DOMINATORS, loop->latch, gimple_bb
>(data->stmt))
>>        && scev_probably_wraps_p (initial_condition_in_loop_num (ev,
>loop->num),
>>  				step, data->stmt, loop, true))
>> -    reliable = false;
>> +    upper = false;
>>  
>> -  record_nonwrapping_iv (loop, init, step, data->stmt, low, high,
>reliable, upper);
>> +  record_nonwrapping_iv (loop, init, step, data->stmt, low, high,
>false, upper);
>>    return true;
>>  }
>>  
>> Index: tree-ssa-loop-unswitch.c
>> ===================================================================
>> --- tree-ssa-loop-unswitch.c	(revision 234516)
>> +++ tree-ssa-loop-unswitch.c	(working copy)
>> @@ -223,6 +223,8 @@ tree_unswitch_single_loop (struct loop *
>>        /* If the loop is not expected to iterate, there is no need
>>  	 for unswitching.  */
>>        iterations = estimated_loop_iterations_int (loop);
>> +      if (iterations < 0)
>> +        iterations = max_loop_iterations_int (loop);
>>        if (iterations >= 0 && iterations <= 1)
>>  	{
>>  	  if (dump_file && (dump_flags & TDF_DETAILS))
>> @@ -439,6 +441,8 @@ tree_unswitch_outer_loop (struct loop *l
>>    /* If the loop is not expected to iterate, there is no need
>>        for unswitching.  */
>>    iterations = estimated_loop_iterations_int (loop);
>> +  if (iterations < 0)
>> +    iterations = max_loop_iterations_int (loop);
>>    if (iterations >= 0 && iterations <= 1)
>>      {
>>        if (dump_file && (dump_flags & TDF_DETAILS))
>> Index: tree-vect-loop.c
>> ===================================================================
>> --- tree-vect-loop.c	(revision 234516)
>> +++ tree-vect-loop.c	(working copy)
>> @@ -2063,6 +2063,8 @@ start_over:
>>  
>>    estimated_niter
>>      = estimated_stmt_executions_int (LOOP_VINFO_LOOP (loop_vinfo));
>> +  if (estimated_niter != -1)
>> +    estimated_niter = max_niter;
>>    if (estimated_niter != -1
>>        && ((unsigned HOST_WIDE_INT) estimated_niter
>>            <= MAX (th, (unsigned)min_profitable_estimate)))
>> 
>> 


  reply	other threads:[~2016-03-30 18:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-30 11:02 Jan Hubicka
2016-03-30 12:09 ` Richard Biener
2016-03-30 12:36   ` Jan Hubicka
2016-03-30 12:49     ` Richard Biener
2016-03-30 18:52       ` Bernhard Reutner-Fischer [this message]
2016-04-07 14:52       ` Tom de Vries
2016-03-30 13:50 ` Bin.Cheng
2016-03-30 14:41   ` Jan Hubicka
2016-03-30 15:30     ` Bin.Cheng

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=64947530-A439-42B4-826F-DA52AF33686D@gmail.com \
    --to=rep.dot.nop@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=rguenther@suse.de \
    /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).