Re: [patch] 4.1 ivopts bug

[Roger Sayle <roger at eyesopen dot com>]

On Fri, 31 Mar 2006, Paul Brook wrote:
> 2006-03-31  Paul Brook  <paul@codesourcery.com>
>
> 	Backport form mainline.
> gcc/
> 	2006-01-14  Zdenek Dvorak <dvorakz@suse.cz>
> 	* tree-ssa-loop-niter.c (number_of_iterations_cond): Split into several
> 	functions.
> 	(number_of_iterations_ne, number_of_iterations_lt_to_ne,
> 	assert_no_overflow_lt, assert_loop_rolls_lt, number_of_iterations_lt,
> 	number_of_iterations_le): New functions.
> 	(number_of_iterations_special): Removed.
> 	(number_of_iterations_exit): Do not use number_of_iterations_special.
> 	* tree.c (unsigned_type_for): Always return integer type.
>
> 	2005-01-06  Zdenek Dvorak <dvorakz@suse.cz>
> 	PR tree-optimization/18527
> 	* tree-ssa-loop-niter.c (number_of_iterations_cond,
> 	number_of_iterations_special, number_of_iterations_exit):
> 	Move base and step of an iv to a single structure.  Add
> 	no_overflow flag, and use it in # of iterations analysis.
> 	* tree-scalar-evolution.c (analyze_scalar_evolution_in_loop): Add
> 	folded_casts argument.
> 	(simple_iv): Pass base and step in a structure.  Set no_overflow
> 	flag.
> 	(scev_const_prop): Add argument to analyze_scalar_evolution_in_loop.
> 	Evaluate expensiveness of computing # of iterations instead of
> 	the final expression.
> 	* tree-scalar-evolution.h (affine_iv): New structure.
> 	(simple_iv): Declaration changed.
> 	* tree-chrec.c (chrec_apply): Handle chrecs containing symbols.
> 	* tree-ssa-loop-ivopts.c (determine_biv_step, find_givs_in_stmt_scev,
> 	find_givs_in_stmt): Changed due to simple_iv change.
>
> gcc/testsuite/
> 	2005-01-06  Zdenek Dvorak <dvorakz@suse.cz>
> 	* gcc.dg/tree-ssa/loop-15.c: New test.
>
> 	2005-01-14  Zdenek Dvorak <dvorakz@suse.cz>
> 	* gcc.dg/tree-ssa/pr19210-1.c: Update outcome.  Add new test loop.
> 	* gcc.dg/tree-ssa/pr19210-2.c: Ditto.

Hi Paul,

I think I'd prefer Mark to make the ultimate call on this one,
now you've clarified that this fixes a wrong code regression.
My only constructive comment is on the following:


> Tested with cross to arm-none-eabi.
> Ok for 4.1?

This is quite a sizeable backport of notoriously complicated iv-opts
code to a stable release branch.  Any chance that you could test on
one or two more platforms?  A cross-compiler doesn't even confirm
that this change will bootstrap.  I appreciate this code has been on
mainline for a while, but there have also been a significant number
of other loop optimization changes on mainline that may be interacting
with the changes you wish to backport.

I suspect Mark's decision would be easier if you also bootstrapped
and regression tested your patch on one or two primary platforms.

I hope this helps.  Thanks in advance.

Roger
--