Re: PATCH RFC: -fstrict-overflow

["Richard Guenther" <richard dot guenther at gmail dot com>]

On 23 Jan 2007 23:19:43 -0800, Ian Lance Taylor <iant@google.com> wrote:
> Mark Mitchell <mark@codesourcery.com> writes:
>
> > I have a cosmetic comment:
> >
> > > +     && (flag_wrapv
> > > +         || !flag_strict_overflow
> >
> > This pattern appears in a lot of places.  I think it would be nice if
> > you would create a signed_overflow_defined_p() macro (maybe that's not
> > quite the right name) that captures this test.  You have to touch all of
> > these spots anyhow, and it would be nice to capture the thinking behind
> > the conditional.

I agree.

> Yes, you're right.  I thought about that a bit, but I didn't quite
> come up with the right set of names.  There are three different
> variables:
>
>   flag_wrapv: gcc assumes that signed overflow wraps
>   flag_trapv: gcc is supposed to issue a trap on signed overflow
>   flag_strict_overflow: gcc may assume signed overflow is undefined
>
> Different combinations of these variables happen in different places.
> negate_expr_p uses (flag_wrapv && !flag_trapv) in some places,
> (flag_wrapv || !flag_strict_overflow) in others.  build_range_check
> just uses (!flag_wrapv).  maybe_canonicalize_comparison uses
> (flag_wrapv || flag_trapv || !flag_strict_overflow).  fold_comparison
> uses (!flag_wrapv && !flag_trapv), and also (flag_wrapv || flag_trapv
> || !flag_strict_overflow).
>
> Part of the problem may be that flag_trapv is handled inconsistently,
> and it is true that using some macros or inline functions might help
> clarify that.
>
> I guess the choices are:
>
> * signed overflow wraps (flag_wrapv)
> * signed overflow traps (flag_trapv)
> * can't assume anything about signed overflow--must let code which may
>   overflow stand as written (flag_wrapv || !flag_strict_overflow)
> * may assume that signed overflow is undefined (!flag_wrapv &&
>   flag_strict_overflow)
>
> It's not clear to me how -ftrapv interacts with signed overflow
> assumptions.  Does -ftrapv mean that signed overflow is undefined?  Or
> does it mean that we may not touch any operation that may overflow?
> I don't know what the original purpose of -ftrapv was.  It was added
> here:
>     http://gcc.gnu.org/ml/gcc-patches/2000-10/msg00607.html
> but that message sheds no light on the goal of the option.
>
> I guess the most plausible semantics would be for -ftrapv to imply
> that we should not touch any operation which might overflow.  That is,
> -ftrapv -fstrict-overflow would be meaningless.
>
> So I'm currently testing this revision of my earlier patch.

Maybe I'm overlooking some detail, but it looks like -fno-strict-overflow
basically enables the same behavior as -fwrapv does at the places you
changed.  So I assume there are places you didn't touch - which are they,
how did you categorize the cases?

The question that comes up is why do we need -f[no-]strict-overflow if
we have -fwrapv?  Why don't we enable -fwrapv for -O0 and -O1?  Is
this because -fwrapv is a stronger statement than -fno-strict-overflow?
But if it is, does -fno-strict-overflow do anything good about all this
discussion?

I see with the new predicates all is well hidden in implementation details,
and I like this.  So maybe split the patch into the (certainly
non-controversical)
predicate introduction part and the part introducing the new flag?

Thanks!
Richard.


> 2007-01-23  Ian Lance Taylor  <iant@google.com>
>
>         * common.opt: Add fstrict-overflow.
>         * opts.c (decode_options): Set flag_strict_overflow if -O2.
>         * flags.h (TYPE_OVERFLOW_WRAPS): Define.
>         (TYPE_OVERFLOW_UNDEFINED): Define.
>         (TYPE_OVERFLOW_TRAPS): Define.  This replaces TYPE_TRAP_SIGNED.
>         Replace all uses.
>         * tree.h (TYPE_TRAP_SIGNED): Don't define.
>         * fold-const.c (negate_expr_p): Use TYPE_OVERFLOW_UNDEFINED.
>         (fold_negate_expr): Likewise.
>         (make_range): Likewise.
>         (extract_muldiv_1): Likewise.
>         (maybe_canonicalize_comparison): Likewise.
>         (fold_comparison): Likewise.
>         (fold_binary): Likewise.
>         (tree_expr_nonnegative_p): Likewise.
>         (tree_expr_nonzero_p): Likewise.
>         * tree-vrp.c (compare_values): Likewise.
>         (extract_range_from_binary_expr): Likewise.
>         (extract_range_from_unary_expr): Likewise.
>         * tree-ssa-loop-niter.c (infer_loop_bounds_from_signedness):
>         Likewise.
>         (nowrap_type_p): Likewise.
>         * tree-scalar-evolution.c (simple_iv): Likewise.
>         * simplify-rtx.c (simplify_const_relational_operation): Check
>         flag_strict_overflow and flag_trapv.
>         (simplify_const_relational_operation): Likewise.
>         * doc/invoke.texi (Option Summary): Mention -fstrict-overflow.
>         (Optimize Options): Add -fstrict-overflow to -O2 list.  Document
>         -fstrict-overflow.