On Thu, Feb 28, 2013 at 07:27:48PM +0100, Marek Polacek wrote:
> The hunk
> probability = (double) REG_BR_PROB_BASE * compare_count / loop_count;
> in there should be probably handled in the same way. But I'll handle
> that separately.
>
> In the first hunk, I did s/int/HOST_WIDE_INT/, because HWI is what
> tree_low_cst returns. For why that could be a problem, see the
> Jakub's comment in the PR.
That doesn't help, because it is assigned into
int *loop_step;
*loop_step = step;
IMHO the argument should be tree *loop_step, then you don't need to
convert it back from int to tree.
> --- gcc/predict.c.mp 2013-02-28 17:26:47.950247877 +0100
> +++ gcc/predict.c 2013-02-28 17:26:56.855275792 +0100
> + /* We want to do the arithmetics on trees (and punt in
> + case of an overflow). */
> + step_var = build_int_cst (NULL_TREE, compare_step);
> + gcc_assert (TREE_CODE (step_var) == INTEGER_CST);
See above, this would be unnecessary.
> +
> + /* Compute (loop_bound - base) / compare_step. */
> + loop_count_var
> + = int_const_binop (TRUNC_DIV_EXPR,
> + int_const_binop (MINUS_EXPR,
> + loop_bound_var,
> + compare_base),
> + step_var);
> +
> + if (TREE_OVERFLOW_P (loop_count_var))
> + return;
> +
> + HOST_WIDE_INT loop_count = tree_low_cst (loop_count_var, 0);
I thought you'd do the rest of the computations on trees too,
there are the risks of overflows or undefined behavior.
So if ((tree_int_cst_sgn (compare_step) == 1)
etc., integer_zerop (loop_count), and the only conversion from tree
to HWI would be after you convert the floating point tree to integer
when you assign it to probability.
Jakub
