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