Re: [patch] for PR 18316 (updated)

[Roger Sayle <roger at eyesopen dot com>]

On Mon, 18 Apr 2005, Zdenek Dvorak wrote:
>
> 	PR tree-optimization/18316
> 	* tree.c (build_int_cst_type): Avoid shift by size of type.
> 	* tree-scalar-evolution.c (simple_iv): Add allow_nonconstant_step
> 	argument.
> 	* tree-scalar-evolution.h (simple_iv): Declaration changed.
> 	* tree-ssa-loop-ivopts.c (struct iv_cand): Add depends_on
> 	field.
> 	(dump_cand): Dump depends_on information.
> 	(determine_biv_step): Add argument to simple_iv call.
> 	(contains_abnormal_ssa_name_p): Handle case expr == NULL.
> 	(find_bivs, find_givs_in_stmt_scev): Do not require step to be a
> 	constant.
> 	(add_candidate_1): Record depends_on for candidates.
> 	(tree_int_cst_sign_bit, constant_multiple_of): New functions.
> 	(get_computation_at, get_computation_cost_at, may_eliminate_iv):
> 	Handle ivs with nonconstant step.
> 	(iv_ca_set_remove_invariants, iv_ca_set_add_invariants): New functions.
> 	(iv_ca_set_no_cp, iv_ca_set_cp): Handle cand->depends_on.
> 	(create_new_iv): Unshare the step before passing it to create_iv.
> 	(free_loop_data): Free cand->depends_on.
> 	* tree-ssa-loop-manip.c (create_iv): Handle ivs with nonconstant step.
> 	* tree-ssa-loop-niter.c (number_of_iterations_exit): Add argument
> 	to simple_iv call.

Now that you've confirmed that this also fixes PR19126 remember to
also mention "PR tree-optimization/19126" in the ChangeLog entry.


> !   if (allow_nonconstant_step)
> !     {
> !       if (tree_contains_chrecs (*step)
> ! 	  || chrec_contains_symbols_defined_in_loop (*step, loop->num))
> ! 	return false;
> !     }
> !   else
> !     {
> !       if (TREE_CODE (*step) != INTEGER_CST)
> ! 	return false;
> !     }

Perhaps better written as

	else if (TREE_CODE (*step) != INTEGER_CST)
	  return false;


> !   step = fold (build2 (MULT_EXPR, type, step, iv_step));
>
>     if (!*dta->step_p)
>       *dta->step_p = step;
>     else
> !     *dta->step_p = fold (build2 (PLUS_EXPR, type, *dta->step_p, step));

These should use the new fold_build2 functions.


The rest of the patch looks reasonable.  Do you have any benchmark
figures on the impact of this patch?  We've had problems in the past
with the number of induction variables we find and their effect on
register pressure.  If there's no (large) performance degradation on
SPEC with this change, I'll approve it for mainline.

Thanks in advance,

Roger
--