From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gcc-patches-return-286328-listarch-gcc-patches=gcc.gnu.org@gcc.gnu.org>
Received: (qmail 17899 invoked by alias); 4 Mar 2011 22:37:14 -0000
Received: (qmail 17702 invoked by uid 22791); 4 Mar 2011 22:37:12 -0000
X-SWARE-Spam-Status: No, hits=-2.0 required=5.0	tests=AWL,BAYES_00,T_RP_MATCHES_RCVD
X-Spam-Check-By: sourceware.org
Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16)    by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Mar 2011 22:37:06 +0000
Received: from localhost (occam.kam.mff.cuni.cz [195.113.17.166])	by nikam.ms.mff.cuni.cz (Postfix) with ESMTP id 7403E9AC80F;	Fri,  4 Mar 2011 23:37:04 +0100 (CET)
Received: by localhost (Postfix, from userid 29025)	id 6EF615640FC; Fri,  4 Mar 2011 23:37:04 +0100 (CET)
Date: Fri, 04 Mar 2011 22:37:00 -0000
From: Zdenek Dvorak <rakdver@kam.mff.cuni.cz>
To: Tom de Vries <vries@codesourcery.com>
Cc: Paolo Bonzini <bonzini@gnu.org>,	Richard Guenther <richard.guenther@gmail.com>,	gcc-patches@gcc.gnu.org, Bernd Schmidt <bernds@codesourcery.com>,	Maxim Kuvyrkov <maxim@codesourcery.com>
Subject: Re: ivopts improvement
Message-ID: <20110304223704.GA9660@kam.mff.cuni.cz>
References: <4D6B6DB9.7050302@codesourcery.com> <4D6B7AFD.6000201@gnu.org> <4D6BAF92.3030707@codesourcery.com> <4D6BB48A.9090003@gnu.org> <AANLkTi=-0CJ=W7JXYs9sBZw7ic6W0SYU9tmJVr+YHh5J@mail.gmail.com> <20110228181222.GA24295@kam.mff.cuni.cz> <4D6EBE2A.10002@codesourcery.com> <4D6F550B.9060400@gnu.org> <4D6FA591.6060604@codesourcery.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4D6FA591.6060604@codesourcery.com>
User-Agent: Mutt/1.5.18 (2008-05-17)
Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-patches.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-patches/>
List-Post: <mailto:gcc-patches@gcc.gnu.org>
List-Help: <mailto:gcc-patches-help@gcc.gnu.org>
Sender: gcc-patches-owner@gcc.gnu.org
X-SW-Source: 2011-03/txt/msg00241.txt.bz2

Hi,

>    /* Whether the loop body includes any function calls.  */
>    bool body_includes_call;
> +
> +  /* Whether the loop body includes any function calls that possibly have side
> +     effects.  */
> +  bool body_includes_side_effect_call;
>  };
>  
>  /* An assignment of iv candidates to uses.  */
> @@ -456,6 +460,20 @@
>    return exit;
>  }
>  
> +/* Returns true if single_exit (DATA->current_loop) is the only possible exit.
> +   Uses the same logic as loop_only_exit_p.  */

why are you duplicating the functionality, instead of simply caching the result
of loop_only_exit_p?

> +/* Tries to detect
> +     NIT == (use_iv_max < USE->iv->base)
> +            ? 0
> +            : (use_iv_max - USE->iv->base)
> +   where
> +     use_iv_real_base == (USE->iv->base - USE->iv->step)
> +     && CAND->iv->base == base_ptr + use_iv_real_base
> +   and returns the exclusive upper bound for CAND->var_after:
> +     base_ptr + use_iv_max.  */
> +
> +static tree
> +get_lt_bound (struct iv_use *use, struct iv_cand *cand, tree nit)
> +{
...
> +  /* use_iv_real_base == use->iv->base - use->iv->step.  */
> +  use_iv_real_base = fold_build_plus (MINUS_EXPR, use->iv->base, use->iv->step);
> +
> +  /* cand_iv_base.  */
> +
> +  /* cand->iv->base == base_ptr + use_iv_real_base.  */
...
> +  /* 0.  */
...

This function seriously needs better comments.  All that are currently present just
give relations between variables that can be as easily seen from the code (but
do not at all explain what the variables are supposed to mean), or make no sense
(what does the 0. comment mean?)

Otherwise the patch looks ok (but I would still like to see get_lt_bound with proper
comments, currently I don't really understand what happens there),

Zdenek