On Tue, Jul 18, 2017 at 6:05 PM, Marek Polacek <pola...@redhat.com> wrote:
> We ended up in infinite recursion between extract_muldiv_1 and
> fold_plusminus_mult_expr, because one turns this expression into the other
> and the other does the reverse:
>
> ((2147483648 / 0) * 2) + 2 <-> 2 * (2147483648 / 0 + 1)
>
> I tried (unsuccessfully) to fix it in either extract_muldiv_1 or
> fold_plusminus_mult_expr, but in the end I went with just turning (x / 0) + A
> to x / 0 (and similarly for %), because with that undefined division we can do
> anything and this fixes the issue. Any better ideas?
Heh - I looked at this at least twice as well with no conclusive fix...
My final thought was to fold division/modulo by zero to __builtin_trap () but
I didn't get to implement that. I'm not sure if we need to preserve
the behavior
of raising SIGFPE as I think at least the C standard makes it undefined.
OTOH other languages with non-call-exceptions might want to catch division
by zero.
Did you see why the oscillation doesn't happen for
((2147483648 / A) * 2) + 2 <-> 2 * (2147483648 / A + 1)
? What's special for the zero constant as divisor?
Richard.
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-07-18 Marek Polacek <pola...@redhat.com>
>
> PR middle-end/70992
> * fold-const.c (fold_binary_loc): Fold (x / 0) + A to x / 0,
> and (x % 0) + A to x % 0.
>
> * gcc.dg/torture/pr70992.c: New test.
> * gcc.dg/torture/pr70992-2.c: New test.
>
> diff --git gcc/fold-const.c gcc/fold-const.c
> index 1bcbbb58154..9abdc9a8c20 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -9387,6 +9387,12 @@ fold_binary_loc (location_t loc,
> TREE_TYPE (arg0), arg0,
> cst0));
> }
> + /* Adding anything to a division-by-zero makes no sense and
> + can confuse extract_muldiv and fold_plusminus_mult_expr. */
> + else if ((TREE_CODE (arg0) == TRUNC_DIV_EXPR
> + || TREE_CODE (arg0) == TRUNC_MOD_EXPR)
> + && integer_zerop (TREE_OPERAND (arg0, 1)))
> + return fold_convert_loc (loc, type, arg0);
> }
>
> /* Handle (A1 * C1) + (A2 * C2) with A1, A2 or C1, C2 being the same or
> diff --git gcc/testsuite/gcc.dg/torture/pr70992-2.c
> gcc/testsuite/gcc.dg/torture/pr70992-2.c
> index e69de29bb2d..c5d2c5f2683 100644
> --- gcc/testsuite/gcc.dg/torture/pr70992-2.c
> +++ gcc/testsuite/gcc.dg/torture/pr70992-2.c
> @@ -0,0 +1,9 @@
> +/* PR middle-end/70992 */
> +/* { dg-do compile } */
> +
> +unsigned int *od;
> +int
> +fn (void)
> +{
> + return (0 % 0 + 1) * *od * 2; /* { dg-warning "division by zero" } */
> +}
> diff --git gcc/testsuite/gcc.dg/torture/pr70992.c
> gcc/testsuite/gcc.dg/torture/pr70992.c
> index e69de29bb2d..56728e09d1b 100644
> --- gcc/testsuite/gcc.dg/torture/pr70992.c
> +++ gcc/testsuite/gcc.dg/torture/pr70992.c
> @@ -0,0 +1,41 @@
> +/* PR middle-end/70992 */
> +/* { dg-do compile } */
> +
> +typedef unsigned int uint32_t;
> +typedef int int32_t;
> +
> +uint32_t
> +fn (uint32_t so)
> +{
> + return (so + so) * (0x80000000 / 0 + 1); /* { dg-warning "division by
> zero" } */
> +}
> +
> +uint32_t
> +fn5 (uint32_t so)
> +{
> + return (0x80000000 / 0 + 1) * (so + so); /* { dg-warning "division by
> zero" } */
> +}
> +
> +uint32_t
> +fn6 (uint32_t so)
> +{
> + return (0x80000000 / 0 - 1) * (so + so); /* { dg-warning "division by
> zero" } */
> +}
> +
> +uint32_t
> +fn2 (uint32_t so)
> +{
> + return (so + so) * (0x80000000 / 0 - 1); /* { dg-warning "division by
> zero" } */
> +}
> +
> +int32_t
> +fn3 (int32_t so)
> +{
> + return (so + so) * (0x80000000 / 0 + 1); /* { dg-warning "division by
> zero" } */
> +}
> +
> +int32_t
> +fn4 (int32_t so)
> +{
> + return (so + so) * (0x80000000 / 0 - 1); /* { dg-warning "division by
> zero" } */
> +}
>
> Marek
