On Thu, Aug 03, 2017 at 11:05:54AM -0500, Bill Schmidt wrote:
> --- gcc/gimple-ssa-strength-reduction.c (revision 250791)
> +++ gcc/gimple-ssa-strength-reduction.c (working copy)
> @@ -2074,6 +2074,10 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
> {
> tree target_type = TREE_TYPE (gimple_assign_lhs (c->cand_stmt));
> enum tree_code cand_code = gimple_assign_rhs_code (c->cand_stmt);
> + unsigned int prec = TYPE_PRECISION (target_type);
> + tree maxval = (POINTER_TYPE_P (target_type)
> + ? TYPE_MAX_VALUE (sizetype)
> + : TYPE_MAX_VALUE (target_type));
>
> /* It is highly unlikely, but possible, that the resulting
> bump doesn't fit in a HWI. Abandon the replacement
> @@ -2082,6 +2086,17 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
> types but allows for safe negation without twisted logic. */
> if (wi::fits_shwi_p (bump)
> && bump.to_shwi () != HOST_WIDE_INT_MIN
> + /* It is more likely that the bump doesn't fit in the target
> + type, so check whether constraining it to that type changes
> + the value. For a signed type, the value mustn't change.
> + For an unsigned type, the value may only change to a
> + congruent value (for negative bumps). */
> + && (TYPE_UNSIGNED (target_type)
> + || wi::eq_p (bump, wi::ext (bump, prec, SIGNED)))
> + && (!TYPE_UNSIGNED (target_type)
> + || wi::eq_p (bump, wi::ext (bump, prec, UNSIGNED))
> + || wi::eq_p (bump + wi::to_widest (maxval) + 1,
> + wi::ext (bump, prec, UNSIGNED)))
wi::ext makes only sense if used with variable TYPE_SIGNED, when
used with constant, it is better to use wi::sext or wi::zext.
And, wouldn't it be more readable to use:
&& (TYPE_UNSIGNED (target_type)
? (wi::eq_p (bump, wi::zext (bump, prec))
|| wi::eq_p (bump + wi::to_widest (maxval) + 1,
wi::zext (bump, prec)))
: wi::eq_p (bump, wi::sext (bump, prec)))
?
For TYPE_UNSIGNED, do you actually need any restriction?
What kind of bump values are wrong for unsigned types and why?
Jakub
