This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: ivopts improvement
- From: Zdenek Dvorak <rakdver at kam dot mff dot cuni dot cz>
- To: Tom de Vries <vries at codesourcery dot com>
- Cc: Paolo Bonzini <bonzini at gnu dot org>, Richard Guenther <richard dot guenther at gmail dot com>, gcc-patches at gcc dot gnu dot org, Bernd Schmidt <bernds at codesourcery dot com>, Maxim Kuvyrkov <maxim at codesourcery dot com>
- Date: Fri, 4 Mar 2011 23:37:04 +0100
- Subject: Re: ivopts improvement
- 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>
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