| Message ID | 20170725121937.GF3397@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 25, 2017 at 2:19 PM, Marek Polacek <polacek@redhat.com> wrote: > On Thu, Jul 20, 2017 at 08:22:59PM +0200, Richard Biener wrote: >> No, we shouldn't. Maybe trapping ops shouldn't be marked TREE_CONSTANT? >> (Make sure to test with Ada...) > > That works for both testcases, but I can't say I really like the idea of > modifying build2... but it's where the TREE_CONSTANT comes from. Yes, I know. > The tree_could_trap_p bit is because the C++ FE can pass "< x" around, e.g. > a LT_EXPR with the first operand null. Eh... :/ > Bootstrapped/regtested on x86_64-linux (Ada included) and powerpc64le-unknown-linux-gnu, > ok for trunk? Eric, any comments? We could also avoid tree_could_trap_p by special-casing only *_DIV_EXPR and *_MOD_EXPR with integer zero 2nd operand explicitely in build2 given there's no "constant" value for this. That is, for FP 1./0. is NaN (a "constant" value) even if the operation might trap. Thanks, Richard. > 2017-07-25 Marek Polacek <polacek@redhat.com> > > PR c/70992 > * tree-eh.c (tree_could_trap_p): Check if the first operand is null. > * tree.c: Include "tree-eh.h". > (build2_stat): Only set TREE_CONSTANT if the tree couldn't trap. > > * gcc.dg/overflow-warn-1.c: Adjust dg-error. > * gcc.dg/overflow-warn-2.c: Likewise. > * gcc.dg/overflow-warn-3.c: Likewise. > * gcc.dg/overflow-warn-4.c: Likewise. > * gcc.dg/torture/pr70992-2.c: New test. > * gcc.dg/torture/pr70992.c: New test. > > diff --git gcc/testsuite/gcc.dg/overflow-warn-1.c gcc/testsuite/gcc.dg/overflow-warn-1.c > index 8eb322579cf..a5cd5738636 100644 > --- gcc/testsuite/gcc.dg/overflow-warn-1.c > +++ gcc/testsuite/gcc.dg/overflow-warn-1.c > @@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "25:integer overflow in expression" > void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */ > /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-1 } */ > void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ > -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */ > +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */ > /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ > void *r = (1 ? 0 : INT_MAX+1); > > diff --git gcc/testsuite/gcc.dg/overflow-warn-2.c gcc/testsuite/gcc.dg/overflow-warn-2.c > index f048d6dae2a..05ab104fa4a 100644 > --- gcc/testsuite/gcc.dg/overflow-warn-2.c > +++ gcc/testsuite/gcc.dg/overflow-warn-2.c > @@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "integer overflow in expression" } > void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */ > /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-1 } */ > void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ > -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */ > +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */ > /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ > void *r = (1 ? 0 : INT_MAX+1); > > diff --git gcc/testsuite/gcc.dg/overflow-warn-3.c gcc/testsuite/gcc.dg/overflow-warn-3.c > index 664011e401d..fd4a34f67e2 100644 > --- gcc/testsuite/gcc.dg/overflow-warn-3.c > +++ gcc/testsuite/gcc.dg/overflow-warn-3.c > @@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } > /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } .-1 } */ > /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ > void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ > -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */ > +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */ > /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ > void *r = (1 ? 0 : INT_MAX+1); > > diff --git gcc/testsuite/gcc.dg/overflow-warn-4.c gcc/testsuite/gcc.dg/overflow-warn-4.c > index 52677ce897a..018e3e1e4cd 100644 > --- gcc/testsuite/gcc.dg/overflow-warn-4.c > +++ gcc/testsuite/gcc.dg/overflow-warn-4.c > @@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } > /* { dg-error "overflow in constant expression" "constant" { target *-*-* } .-1 } */ > /* { dg-error "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ > void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ > -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */ > +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */ > /* { dg-error "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ > void *r = (1 ? 0 : INT_MAX+1); > > 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" } */ > +} > diff --git gcc/tree-eh.c gcc/tree-eh.c > index c68d71a5e54..cde4ef3f387 100644 > --- gcc/tree-eh.c > +++ gcc/tree-eh.c > @@ -2599,7 +2599,7 @@ tree_could_trap_p (tree expr) > > if (t) > { > - if (COMPARISON_CLASS_P (expr)) > + if (COMPARISON_CLASS_P (expr) && TREE_OPERAND (expr, 0)) > fp_operation = FLOAT_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 0))); > else > fp_operation = FLOAT_TYPE_P (t); > diff --git gcc/tree.c gcc/tree.c > index b7de2840ac6..205ba7fd79d 100644 > --- gcc/tree.c > +++ gcc/tree.c > @@ -62,6 +62,7 @@ along with GCC; see the file COPYING3. If not see > #include "print-tree.h" > #include "ipa-utils.h" > #include "selftest.h" > +#include "tree-eh.h" > > /* Tree code classes. */ > > @@ -4505,7 +4506,7 @@ build2_stat (enum tree_code code, tree tt, tree arg0, tree arg1 MEM_STAT_DECL) > else > { > TREE_READONLY (t) = read_only; > - TREE_CONSTANT (t) = constant; > + TREE_CONSTANT (t) = constant && !tree_could_trap_p (t); > TREE_THIS_VOLATILE (t) > = (TREE_CODE_CLASS (code) == tcc_reference > && arg0 && TREE_THIS_VOLATILE (arg0)); > > Marek
> Eric, any comments? No objection for the build2_stat hunk, I think it's in keeping with the Ada semantics. But the tree_could_trap_p hunk is certainly an abomination... > We could also avoid tree_could_trap_p by special-casing only > *_DIV_EXPR and *_MOD_EXPR > with integer zero 2nd operand explicitely in build2 given there's no > "constant" value for this. That is, > for FP 1./0. is NaN (a "constant" value) even if the operation might trap. Yes, that would be faster & simpler and avoids the abomination.
On Tue, Jul 25, 2017 at 3:30 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> Eric, any comments? > > No objection for the build2_stat hunk, I think it's in keeping with the Ada > semantics. But the tree_could_trap_p hunk is certainly an abomination... > >> We could also avoid tree_could_trap_p by special-casing only >> *_DIV_EXPR and *_MOD_EXPR >> with integer zero 2nd operand explicitely in build2 given there's no >> "constant" value for this. That is, >> for FP 1./0. is NaN (a "constant" value) even if the operation might trap. > > Yes, that would be faster & simpler and avoids the abomination. Ok, then let's go with that slightly uglier but less abominal variant ;) Thanks, Richard. > -- > Eric Botcazou
diff --git gcc/testsuite/gcc.dg/overflow-warn-1.c gcc/testsuite/gcc.dg/overflow-warn-1.c index 8eb322579cf..a5cd5738636 100644 --- gcc/testsuite/gcc.dg/overflow-warn-1.c +++ gcc/testsuite/gcc.dg/overflow-warn-1.c @@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "25:integer overflow in expression" void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */ /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-1 } */ void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */ +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */ /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ void *r = (1 ? 0 : INT_MAX+1); diff --git gcc/testsuite/gcc.dg/overflow-warn-2.c gcc/testsuite/gcc.dg/overflow-warn-2.c index f048d6dae2a..05ab104fa4a 100644 --- gcc/testsuite/gcc.dg/overflow-warn-2.c +++ gcc/testsuite/gcc.dg/overflow-warn-2.c @@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "integer overflow in expression" } void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */ /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-1 } */ void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */ +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */ /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ void *r = (1 ? 0 : INT_MAX+1); diff --git gcc/testsuite/gcc.dg/overflow-warn-3.c gcc/testsuite/gcc.dg/overflow-warn-3.c index 664011e401d..fd4a34f67e2 100644 --- gcc/testsuite/gcc.dg/overflow-warn-3.c +++ gcc/testsuite/gcc.dg/overflow-warn-3.c @@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } .-1 } */ /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */ +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */ /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ void *r = (1 ? 0 : INT_MAX+1); diff --git gcc/testsuite/gcc.dg/overflow-warn-4.c gcc/testsuite/gcc.dg/overflow-warn-4.c index 52677ce897a..018e3e1e4cd 100644 --- gcc/testsuite/gcc.dg/overflow-warn-4.c +++ gcc/testsuite/gcc.dg/overflow-warn-4.c @@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } /* { dg-error "overflow in constant expression" "constant" { target *-*-* } .-1 } */ /* { dg-error "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */ +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */ /* { dg-error "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */ void *r = (1 ? 0 : INT_MAX+1); 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" } */ +} diff --git gcc/tree-eh.c gcc/tree-eh.c index c68d71a5e54..cde4ef3f387 100644 --- gcc/tree-eh.c +++ gcc/tree-eh.c @@ -2599,7 +2599,7 @@ tree_could_trap_p (tree expr) if (t) { - if (COMPARISON_CLASS_P (expr)) + if (COMPARISON_CLASS_P (expr) && TREE_OPERAND (expr, 0)) fp_operation = FLOAT_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 0))); else fp_operation = FLOAT_TYPE_P (t); diff --git gcc/tree.c gcc/tree.c index b7de2840ac6..205ba7fd79d 100644 --- gcc/tree.c +++ gcc/tree.c @@ -62,6 +62,7 @@ along with GCC; see the file COPYING3. If not see #include "print-tree.h" #include "ipa-utils.h" #include "selftest.h" +#include "tree-eh.h" /* Tree code classes. */ @@ -4505,7 +4506,7 @@ build2_stat (enum tree_code code, tree tt, tree arg0, tree arg1 MEM_STAT_DECL) else { TREE_READONLY (t) = read_only; - TREE_CONSTANT (t) = constant; + TREE_CONSTANT (t) = constant && !tree_could_trap_p (t); TREE_THIS_VOLATILE (t) = (TREE_CODE_CLASS (code) == tcc_reference && arg0 && TREE_THIS_VOLATILE (arg0));