Richard Biener <richard.guent...@gmail.com> writes:
> On Mon, Jun 1, 2015 at 10:06 PM, Richard Sandiford
> <rdsandif...@googlemail.com> wrote:
>> Marek Polacek <pola...@redhat.com> writes:
>>> + /* Left-hand operand must be signed. */
>>> + if (TYPE_UNSIGNED (type0))
>>> + return false;
>>> +
>>> + /* Compute the result in infinite precision math (sort of). */
>>> + widest_int w = wi::lshift (wi::to_widest (op0), wi::to_widest (op1));
>>> + unsigned int min_prec = wi::min_precision (w, SIGNED);
>>> + /* Watch out for shifting a negative value. */
>>> + tree r = wide_int_to_tree (tree_int_cst_sgn (op0) >= 0
>>> + ? unsigned_type_for (type0)
>>> + : type0, w);
>>> + bool overflowed = wi::cmps (w, wi::to_widest (r));
>>> + if (overflowed && c_inhibit_evaluation_warnings == 0)
>>> + warning_at (loc, OPT_Wshift_overflow,
>>> + "result of %qE requires %u bits to represent, "
>>> + "but %qT only has %u bits",
>>> + build2_loc (loc, LSHIFT_EXPR, type0, op0, op1),
>>> + min_prec, type0, TYPE_PRECISION (type0));
>>> +
>>> + return overflowed;
>>
>> Yeah, this "sort of" is a bit worrying :-) Especially as the number
>> of bits in a widest_int depends on the number of bits in the target's
>> widest integer mode. E.g. for i386 it's forced to 128, but for ARM
>> it's 512 (IIRC).
>>
>> Could you do the check based on the wi::min_precision of the unshifted
>> value? I.e. see whether adding the shift amount to that gives a value
>> greater than type's precision.
>
> You could always use a FIXED_WIDE_INT like VRP does for its overflow
> detection stuff.
That would work too, but why impose an arbitrary limit? Unless I'm
missing something, the code above should be equivalent to:
unsigned int min_prec = (wi::min_precision (op0, SIGNED)
+ TREE_INT_CST_LOW (op1));
bool overflowed = min_prec > TYPE_PRECISION (type0);
if (overflowed && c_inhibit_evaluation_warnings == 0)
warning_at (loc, OPT_Wshift_overflow,
"result of %qE requires %u bits to represent, "
"but %qT only has %u bits",
build2_loc (loc, LSHIFT_EXPR, type0, op0, op1),
min_prec, type0, TYPE_PRECISION (type0));
which seems simpler than anything involving wider precision.
Thanks,
Richard
