This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: VRP: undefined shifting calculation should not need sign bit
- From: Richard Sandiford <richard dot sandiford at arm dot com>
- To: Aldy Hernandez <aldyh at redhat dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 12 Sep 2018 17:57:40 +0100
- Subject: Re: VRP: undefined shifting calculation should not need sign bit
- References: <1ab7411f-d602-5aa6-4b4a-02c066d693a3@redhat.com>
Aldy Hernandez <aldyh@redhat.com> writes:
> diff --git a/gcc/wide-int-range.h b/gcc/wide-int-range.h
> index 589fdea4df6..e9ee418e5b2 100644
> --- a/gcc/wide-int-range.h
> +++ b/gcc/wide-int-range.h
> @@ -131,7 +131,7 @@ extern bool wide_int_range_div (wide_int &wmin, wide_int &wmax,
> /* Return TRUE if shifting by range [MIN, MAX] is undefined behavior. */
>
> inline bool
> -wide_int_range_shift_undefined_p (signop sign, unsigned prec,
> +wide_int_range_shift_undefined_p (unsigned prec,
> const wide_int &min, const wide_int &max)
> {
> /* ?? Note: The original comment said this only applied to
> @@ -142,7 +142,7 @@ wide_int_range_shift_undefined_p (signop sign, unsigned prec,
> behavior from the shift operation. We cannot even trust
> SHIFT_COUNT_TRUNCATED at this stage, because that applies to rtl
> shifts, and the operation at the tree level may be widened. */
> - return wi::lt_p (min, 0, sign) || wi::ge_p (max, prec, sign);
> + return wi::sign_mask (min) || wi::ge_p (max, prec, UNSIGNED);
I don't think this is a good idea. Logically the comparison should
be done relative to the TYPE_SIGN of the type, so I think the original
code was correct.
Thanks,
Richard