* [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently @ 2016-06-08 10:42 Richard Biener 2016-06-08 15:16 ` Marc Glisse 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2016-06-08 10:42 UTC (permalink / raw) To: gcc-patches The following works around PR70992 but the issue came up repeatedly that we are not very consistent in preserving the undefined behavior of division or modulo by zero. Ok - the only inconsistency is that we fold 0 % x to 0 but not 0 % 0 (with literal zero). After folding is now no longer done early in the C family FEs the number of diagnostic regressions with the patch below is two. FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) FAIL: gcc.dg/wcaselabel-1.c (test for errors, line 10) And then there is a -fnon-call-exceptions testcase FAIL: gcc.c-torture/execute/20101011-1.c -O1 execution test FAIL: gcc.c-torture/execute/20101011-1.c -O2 execution test FAIL: gcc.c-torture/execute/20101011-1.c -O2 -flto -fno-use-linker-plugin -flt o-partition=none execution test FAIL: gcc.c-torture/execute/20101011-1.c -O2 -flto -fuse-linker-plugin -fno-fa t-lto-objects execution test FAIL: gcc.c-torture/execute/20101011-1.c -O3 -g execution test FAIL: gcc.c-torture/execute/20101011-1.c -Os execution test which tests that 0/0 traps (on targets where it does). This shows we might want to guard the simplifications against -fnon-call-exceptions. The other way to fix the inconsistency is of course to not rely on undefinedness in 0 % x simplification and disable that if x is not known to be nonzero. We can introduce the other transforms with properly guarding against a zero 2nd operand as well. So - any opinions here? Thanks, Richard. FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) FAIL: gcc.c-torture/execute/20101011-1.c -O1 execution test FAIL: gcc.c-torture/execute/20101011-1.c -O2 execution test FAIL: gcc.c-torture/execute/20101011-1.c -O2 -flto -fno-use-linker-plugin -flt o-partition=none execution test FAIL: gcc.c-torture/execute/20101011-1.c -O2 -flto -fuse-linker-plugin -fno-fa t-lto-objects execution test FAIL: gcc.c-torture/execute/20101011-1.c -O3 -g execution test FAIL: gcc.c-torture/execute/20101011-1.c -Os execution test FAIL: gcc.dg/wcaselabel-1.c (test for errors, line 10) 2016-06-08 Richard Biener <rguenther@suse.de> PR middle-end/70992 * match.pd (X / X -> 1): Add. (0 / X -> 0): Likewise. (0 % X -> 0): Remove restriction on X not being literal zero. Index: gcc/match.pd =================================================================== --- gcc/match.pd (revision 237205) +++ gcc/match.pd (working copy) @@ -140,12 +140,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) || !COMPLEX_FLOAT_TYPE_P (type))) (negate @0))) -/* Make sure to preserve divisions by zero. This is the reason why - we don't simplify x / x to 1 or 0 / x to 0. */ (for op (mult trunc_div ceil_div floor_div round_div exact_div) (simplify (op @0 integer_onep) (non_lvalue @0))) +/* Make sure to preserve divisions by zero. This is the reason why + we don't simplify x / x to 1 or 0 / x to 0. */ +(for op (trunc_div ceil_div floor_div round_div exact_div) + (simplify + (op @0 @0) + { build_one_cst (type); }) + (simplify + (op integer_zerop@0 @1) + @0)) /* X / -1 is -X. */ (for div (trunc_div ceil_div floor_div round_div exact_div) @@ -255,9 +262,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* 0 % X is always zero. */ (simplify (mod integer_zerop@0 @1) - /* But not for 0 % 0 so that we can get the proper warnings and errors. */ - (if (!integer_zerop (@1)) - @0)) + @0) /* X % 1 is always zero. */ (simplify (mod @0 integer_onep) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-08 10:42 [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently Richard Biener @ 2016-06-08 15:16 ` Marc Glisse 2016-06-08 17:44 ` Jason Merrill 0 siblings, 1 reply; 13+ messages in thread From: Marc Glisse @ 2016-06-08 15:16 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches On Wed, 8 Jun 2016, Richard Biener wrote: > The following works around PR70992 but the issue came up repeatedly > that we are not very consistent in preserving the undefined behavior > of division or modulo by zero. Ok - the only inconsistency is > that we fold 0 % x to 0 but not 0 % 0 (with literal zero). > > After folding is now no longer done early in the C family FEs the > number of diagnostic regressions with the patch below is two. > > FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) > FAIL: gcc.dg/wcaselabel-1.c (test for errors, line 10) > > And then there is a -fnon-call-exceptions testcase > > FAIL: gcc.c-torture/execute/20101011-1.c -O1 execution test > FAIL: gcc.c-torture/execute/20101011-1.c -O2 execution test > FAIL: gcc.c-torture/execute/20101011-1.c -O2 -flto > -fno-use-linker-plugin -flt > o-partition=none execution test > FAIL: gcc.c-torture/execute/20101011-1.c -O2 -flto -fuse-linker-plugin > -fno-fa > t-lto-objects execution test > FAIL: gcc.c-torture/execute/20101011-1.c -O3 -g execution test > FAIL: gcc.c-torture/execute/20101011-1.c -Os execution test > > which tests that 0/0 traps (on targets where it does). This shows > we might want to guard the simplifications against -fnon-call-exceptions. > > The other way to fix the inconsistency is of course to not rely > on undefinedness in 0 % x simplification and disable that if x > is not known to be nonzero. We can introduce the other transforms > with properly guarding against a zero 2nd operand as well. > > So - any opinions here? Note that currently, VRP optimizes 0/0 to 0 (but not 0%0) when we don't pass -fnon-call-exceptions. If we guard with !flag_non_call_exceptions || tree_expr_nonzero_p(...), the transformation seems safe for the middle-end, so the main issue is front-ends. A few random ideas I was considering: * restrict it to GIMPLE, so we can't have a regression in the front-ends. * fold x/0 to 0 with TREE_OVERFLOW set, to tell the front-end that something is going on. * fold to (x/y,0) or (x/y,1) so the division by 0 is still there, but C++11 constexpr might give a strange message about it, and folding might not be idempotent. But as long as we don't always perform the simplification, we need some other way to break the cycle for PR70992. -- Marc Glisse ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-08 15:16 ` Marc Glisse @ 2016-06-08 17:44 ` Jason Merrill 2016-06-08 17:52 ` Jakub Jelinek 2016-06-09 6:50 ` Richard Biener 0 siblings, 2 replies; 13+ messages in thread From: Jason Merrill @ 2016-06-08 17:44 UTC (permalink / raw) To: gcc-patches List; +Cc: Richard Biener On Wed, Jun 8, 2016 at 11:16 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Wed, 8 Jun 2016, Richard Biener wrote: > >> The following works around PR70992 but the issue came up repeatedly >> that we are not very consistent in preserving the undefined behavior >> of division or modulo by zero. Ok - the only inconsistency is >> that we fold 0 % x to 0 but not 0 % 0 (with literal zero). >> >> After folding is now no longer done early in the C family FEs the >> number of diagnostic regressions with the patch below is two. >> >> FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) Yep. We don't want to fold away undefined behavior in a constexpr function, since constexpr evaluation wants to detect undefined behavior and treat the expression as non-constant in that case. > A few random ideas I was considering: > * restrict it to GIMPLE, so we can't have a regression in the front-ends. > * fold x/0 to 0 with TREE_OVERFLOW set, to tell the front-end that something > is going on. > * fold to (x/y,0) or (x/y,1) so the division by 0 is still there, but C++11 > constexpr might give a strange message about it, and folding might not be > idempotent. Any of these would avoid the constexpr regression, though the second would make the diagnostic worse. Or the front end could copy constexpr function bodies before folding. Jason ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-08 17:44 ` Jason Merrill @ 2016-06-08 17:52 ` Jakub Jelinek 2016-06-08 22:18 ` Marc Glisse 2016-06-09 6:50 ` Richard Biener 1 sibling, 1 reply; 13+ messages in thread From: Jakub Jelinek @ 2016-06-08 17:52 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Richard Biener On Wed, Jun 08, 2016 at 01:43:56PM -0400, Jason Merrill wrote: > > A few random ideas I was considering: > > * restrict it to GIMPLE, so we can't have a regression in the front-ends. > > * fold x/0 to 0 with TREE_OVERFLOW set, to tell the front-end that something > > is going on. > > * fold to (x/y,0) or (x/y,1) so the division by 0 is still there, but C++11 > > constexpr might give a strange message about it, and folding might not be > > idempotent. > > Any of these would avoid the constexpr regression, though the second > would make the diagnostic worse. Or the front end could copy > constexpr function bodies before folding. Or, both cxx_eval_binary_expression and cp_fold would need to not fold if the divisor is integer_zerop. Jakub ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-08 17:52 ` Jakub Jelinek @ 2016-06-08 22:18 ` Marc Glisse 2016-06-09 6:53 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Marc Glisse @ 2016-06-08 22:18 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches List, Richard Biener On Wed, 8 Jun 2016, Jakub Jelinek wrote: > On Wed, Jun 08, 2016 at 01:43:56PM -0400, Jason Merrill wrote: >>> A few random ideas I was considering: >>> * restrict it to GIMPLE, so we can't have a regression in the front-ends. >>> * fold x/0 to 0 with TREE_OVERFLOW set, to tell the front-end that something >>> is going on. >>> * fold to (x/y,0) or (x/y,1) so the division by 0 is still there, but C++11 >>> constexpr might give a strange message about it, and folding might not be >>> idempotent. >> >> Any of these would avoid the constexpr regression, though the second >> would make the diagnostic worse. Or the front end could copy >> constexpr function bodies before folding. > > Or, both cxx_eval_binary_expression and cp_fold would need to > not fold if the divisor is integer_zerop. Ah, right, I think I understand the current condition for folding 0%X now. It is meant for warnings, not validity, and with delayed folding, we can only have issues with 0%0, we cannot miss warnings or constexpr-UB by folding 0%X, since if we were going to discover that X is 0 early enough to warn (or in constexpr evaluation) we would have discovered it before the call to fold. If I got it right, that means the condition could actually be, instead of !integer_zerop (@1): flag_non_call_exceptions ? tree_expr_nonzero_p (@1) : (GIMPLE || !integer_zerop (@1)) (or something similar at a different level in the call chain of course) (not that this helps in any way for the PR...) -- Marc Glisse ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-08 22:18 ` Marc Glisse @ 2016-06-09 6:53 ` Richard Biener 0 siblings, 0 replies; 13+ messages in thread From: Richard Biener @ 2016-06-09 6:53 UTC (permalink / raw) To: gcc-patches List; +Cc: Jakub Jelinek, Jason Merrill On Thu, 9 Jun 2016, Marc Glisse wrote: > On Wed, 8 Jun 2016, Jakub Jelinek wrote: > > > On Wed, Jun 08, 2016 at 01:43:56PM -0400, Jason Merrill wrote: > > > > A few random ideas I was considering: > > > > * restrict it to GIMPLE, so we can't have a regression in the > > > > front-ends. > > > > * fold x/0 to 0 with TREE_OVERFLOW set, to tell the front-end that > > > > something > > > > is going on. > > > > * fold to (x/y,0) or (x/y,1) so the division by 0 is still there, but > > > > C++11 > > > > constexpr might give a strange message about it, and folding might not > > > > be > > > > idempotent. > > > > > > Any of these would avoid the constexpr regression, though the second > > > would make the diagnostic worse. Or the front end could copy > > > constexpr function bodies before folding. > > > > Or, both cxx_eval_binary_expression and cp_fold would need to > > not fold if the divisor is integer_zerop. > > Ah, right, I think I understand the current condition for folding 0%X now. It > is meant for warnings, not validity, and with delayed folding, we can only > have issues with 0%0, we cannot miss warnings or constexpr-UB by folding 0%X, > since if we were going to discover that X is 0 early enough to warn (or in > constexpr evaluation) we would have discovered it before the call to fold. > > If I got it right, that means the condition could actually be, instead of > !integer_zerop (@1): > > flag_non_call_exceptions ? tree_expr_nonzero_p (@1) : > (GIMPLE || !integer_zerop (@1)) I think I can live with testing tree_expr_nonzero_p for flag_non_call_exceptions (or just disabling the pattern for flag_non_call_exceptions). But the literal zero or the GIMPLE check isn't what I'd like to see. I want to understand the FE requirements some more and esp. understand why delayed folding makes this still an issue. [as much as the C FE now figures out what is an lvalue or rvalue before folding the FEs should figure out whether sth is a constexpr or not before folding - that should be doable without doing any folding, see my examples in the other mail about undefined behavior] > (or something similar at a different level in the call chain of course) > > (not that this helps in any way for the PR...) Not with -fnon-call-exceptions at least. I just mentioned the PR to get some attention ;) Richard. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-08 17:44 ` Jason Merrill 2016-06-08 17:52 ` Jakub Jelinek @ 2016-06-09 6:50 ` Richard Biener 2016-06-09 7:16 ` Jakub Jelinek 1 sibling, 1 reply; 13+ messages in thread From: Richard Biener @ 2016-06-09 6:50 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List On Wed, 8 Jun 2016, Jason Merrill wrote: > On Wed, Jun 8, 2016 at 11:16 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > > On Wed, 8 Jun 2016, Richard Biener wrote: > > > >> The following works around PR70992 but the issue came up repeatedly > >> that we are not very consistent in preserving the undefined behavior > >> of division or modulo by zero. Ok - the only inconsistency is > >> that we fold 0 % x to 0 but not 0 % 0 (with literal zero). > >> > >> After folding is now no longer done early in the C family FEs the > >> number of diagnostic regressions with the patch below is two. > >> > >> FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) > > Yep. We don't want to fold away undefined behavior in a constexpr > function, since constexpr evaluation wants to detect undefined > behavior and treat the expression as non-constant in that case. Hmm. So 0 / x is not constant because x might be zero but 0 * x is constant because it can never invoke undefined behavior? Does this mean that 0 << n is not const because n might be too large (I suppose 0 << 12000 is not const already)? Is 0 * (-x) const? x might be INT_MIN. So can you clarify on "We don't want to fold away undefined behavior in a constexpr"? Thanks, Richard. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-09 6:50 ` Richard Biener @ 2016-06-09 7:16 ` Jakub Jelinek 2016-06-09 7:39 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Jakub Jelinek @ 2016-06-09 7:16 UTC (permalink / raw) To: Richard Biener; +Cc: Jason Merrill, gcc-patches List On Thu, Jun 09, 2016 at 08:50:15AM +0200, Richard Biener wrote: > On Wed, 8 Jun 2016, Jason Merrill wrote: > > > On Wed, Jun 8, 2016 at 11:16 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > > > On Wed, 8 Jun 2016, Richard Biener wrote: > > > > > >> The following works around PR70992 but the issue came up repeatedly > > >> that we are not very consistent in preserving the undefined behavior > > >> of division or modulo by zero. Ok - the only inconsistency is > > >> that we fold 0 % x to 0 but not 0 % 0 (with literal zero). > > >> > > >> After folding is now no longer done early in the C family FEs the > > >> number of diagnostic regressions with the patch below is two. > > >> > > >> FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) > > > > Yep. We don't want to fold away undefined behavior in a constexpr > > function, since constexpr evaluation wants to detect undefined > > behavior and treat the expression as non-constant in that case. > > Hmm. So 0 / x is not constant because x might be zero but 0 * x is > constant because it can never invoke undefined behavior? Does this mean > that 0 << n is not const because n might be too large (I suppose > 0 << 12000 is not const already)? Is 0 * (-x) const? x might be INT_MIN. E.g. for the shifts the C++ FE has cxx_eval_check_shift_p which should optionally warn and/or set *non_constant_p. 0 * (-INT_MIN) would be non-constant too, etc. constexpr int foo (int x) { return -x; } constexpr int bar (int x) { return 0 * (-x); } constexpr int a = foo (-__INT_MAX__ - 1); constexpr int b = bar (-__INT_MAX__ - 1); shows that we don't diagnose the latter though, most likely because constexpr evaluation is done on the folded tree. So, either whatever cp_fold does (which uses fold* underneath) should avoid folding such cases, or the constexpr evaluation should be done on a copy made before folding. Then cp_fold doesn't have to prohibit optimizations and it is a matter of constexpr.c routines to detect all the undefined behavior. After all, I think doing constexpr evaluation on unfolded trees will have also the advantage of better diagnostic locations. Jakub ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-09 7:16 ` Jakub Jelinek @ 2016-06-09 7:39 ` Richard Biener 2016-06-09 22:07 ` Jason Merrill 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2016-06-09 7:39 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches List On Thu, 9 Jun 2016, Jakub Jelinek wrote: > On Thu, Jun 09, 2016 at 08:50:15AM +0200, Richard Biener wrote: > > On Wed, 8 Jun 2016, Jason Merrill wrote: > > > > > On Wed, Jun 8, 2016 at 11:16 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > > > > On Wed, 8 Jun 2016, Richard Biener wrote: > > > > > > > >> The following works around PR70992 but the issue came up repeatedly > > > >> that we are not very consistent in preserving the undefined behavior > > > >> of division or modulo by zero. Ok - the only inconsistency is > > > >> that we fold 0 % x to 0 but not 0 % 0 (with literal zero). > > > >> > > > >> After folding is now no longer done early in the C family FEs the > > > >> number of diagnostic regressions with the patch below is two. > > > >> > > > >> FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) > > > > > > Yep. We don't want to fold away undefined behavior in a constexpr > > > function, since constexpr evaluation wants to detect undefined > > > behavior and treat the expression as non-constant in that case. > > > > Hmm. So 0 / x is not constant because x might be zero but 0 * x is > > constant because it can never invoke undefined behavior? Does this mean > > that 0 << n is not const because n might be too large (I suppose > > 0 << 12000 is not const already)? Is 0 * (-x) const? x might be INT_MIN. > > E.g. for the shifts the C++ FE has cxx_eval_check_shift_p which should > optionally warn and/or set *non_constant_p. 0 * (-INT_MIN) would be > non-constant too, etc. > constexpr int foo (int x) { return -x; } > constexpr int bar (int x) { return 0 * (-x); } > constexpr int a = foo (-__INT_MAX__ - 1); > constexpr int b = bar (-__INT_MAX__ - 1); > shows that we don't diagnose the latter though, most likely because > constexpr evaluation is done on the folded tree. > So, either whatever cp_fold does (which uses fold* underneath) should avoid > folding such cases, or the constexpr evaluation should be done on a copy > made before folding. Then cp_fold doesn't have to prohibit optimizations > and it is a matter of constexpr.c routines to detect all the undefined > behavior. After all, I think doing constexpr evaluation on unfolded trees > will have also the advantage of better diagnostic locations. Yes, I think constexpr diagnostic / detection should be done on unfolded trees (not sure why we'd need to a copy here, just do folding later?). If cp_fold already avoids folding 0 * (-x) then it should simply avoid folding 0 / x or 0 % x as well (for the particular issue this thread started on). Richard. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-09 7:39 ` Richard Biener @ 2016-06-09 22:07 ` Jason Merrill 2016-06-10 7:11 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Jason Merrill @ 2016-06-09 22:07 UTC (permalink / raw) To: Richard Biener; +Cc: Jakub Jelinek, gcc-patches List On Thu, Jun 9, 2016 at 3:39 AM, Richard Biener <rguenther@suse.de> wrote: > On Thu, 9 Jun 2016, Jakub Jelinek wrote: > >> On Thu, Jun 09, 2016 at 08:50:15AM +0200, Richard Biener wrote: >> > On Wed, 8 Jun 2016, Jason Merrill wrote: >> > >> > > On Wed, Jun 8, 2016 at 11:16 AM, Marc Glisse <marc.glisse@inria.fr> wrote: >> > > > On Wed, 8 Jun 2016, Richard Biener wrote: >> > > > >> > > >> The following works around PR70992 but the issue came up repeatedly >> > > >> that we are not very consistent in preserving the undefined behavior >> > > >> of division or modulo by zero. Ok - the only inconsistency is >> > > >> that we fold 0 % x to 0 but not 0 % 0 (with literal zero). >> > > >> >> > > >> After folding is now no longer done early in the C family FEs the >> > > >> number of diagnostic regressions with the patch below is two. >> > > >> >> > > >> FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) >> > > >> > > Yep. We don't want to fold away undefined behavior in a constexpr >> > > function, since constexpr evaluation wants to detect undefined >> > > behavior and treat the expression as non-constant in that case. >> > >> > Hmm. So 0 / x is not constant because x might be zero but 0 * x is >> > constant because it can never invoke undefined behavior? Does this mean >> > that 0 << n is not const because n might be too large (I suppose >> > 0 << 12000 is not const already)? Is 0 * (-x) const? x might be INT_MIN. >> >> E.g. for the shifts the C++ FE has cxx_eval_check_shift_p which should >> optionally warn and/or set *non_constant_p. 0 * (-INT_MIN) would be >> non-constant too, etc. >> constexpr int foo (int x) { return -x; } >> constexpr int bar (int x) { return 0 * (-x); } >> constexpr int a = foo (-__INT_MAX__ - 1); >> constexpr int b = bar (-__INT_MAX__ - 1); >> shows that we don't diagnose the latter though, most likely because >> constexpr evaluation is done on the folded tree. I don't think there's any folding before constexpr evaluation in this case, since this doesn't involve a call. >> So, either whatever cp_fold does (which uses fold* underneath) should avoid >> folding such cases, or the constexpr evaluation should be done on a copy >> made before folding. Then cp_fold doesn't have to prohibit optimizations >> and it is a matter of constexpr.c routines to detect all the undefined >> behavior. After all, I think doing constexpr evaluation on unfolded trees >> will have also the advantage of better diagnostic locations. > > Yes, I think constexpr diagnostic / detection should be done on > unfolded trees (not sure why we'd need to a copy here, just do folding > later?). If cp_fold already avoids folding 0 * (-x) then it should > simply avoid folding 0 / x or 0 % x as well (for the particular issue > this thread started on). The issue is with constexpr functions, which get delayed folding like any other function before they are used in constant expression evaluation. The copy would be to preserve the unfolded trees through cp_fold_function. I've been thinking about doing this anyway; this may be the motivation to go ahead with it. Jason ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-09 22:07 ` Jason Merrill @ 2016-06-10 7:11 ` Richard Biener 2016-06-10 16:16 ` Jason Merrill 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2016-06-10 7:11 UTC (permalink / raw) To: Jason Merrill; +Cc: Jakub Jelinek, gcc-patches List On Thu, 9 Jun 2016, Jason Merrill wrote: > On Thu, Jun 9, 2016 at 3:39 AM, Richard Biener <rguenther@suse.de> wrote: > > On Thu, 9 Jun 2016, Jakub Jelinek wrote: > > > >> On Thu, Jun 09, 2016 at 08:50:15AM +0200, Richard Biener wrote: > >> > On Wed, 8 Jun 2016, Jason Merrill wrote: > >> > > >> > > On Wed, Jun 8, 2016 at 11:16 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > >> > > > On Wed, 8 Jun 2016, Richard Biener wrote: > >> > > > > >> > > >> The following works around PR70992 but the issue came up repeatedly > >> > > >> that we are not very consistent in preserving the undefined behavior > >> > > >> of division or modulo by zero. Ok - the only inconsistency is > >> > > >> that we fold 0 % x to 0 but not 0 % 0 (with literal zero). > >> > > >> > >> > > >> After folding is now no longer done early in the C family FEs the > >> > > >> number of diagnostic regressions with the patch below is two. > >> > > >> > >> > > >> FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) > >> > > > >> > > Yep. We don't want to fold away undefined behavior in a constexpr > >> > > function, since constexpr evaluation wants to detect undefined > >> > > behavior and treat the expression as non-constant in that case. > >> > > >> > Hmm. So 0 / x is not constant because x might be zero but 0 * x is > >> > constant because it can never invoke undefined behavior? Does this mean > >> > that 0 << n is not const because n might be too large (I suppose > >> > 0 << 12000 is not const already)? Is 0 * (-x) const? x might be INT_MIN. > >> > >> E.g. for the shifts the C++ FE has cxx_eval_check_shift_p which should > >> optionally warn and/or set *non_constant_p. 0 * (-INT_MIN) would be > >> non-constant too, etc. > >> constexpr int foo (int x) { return -x; } > >> constexpr int bar (int x) { return 0 * (-x); } > >> constexpr int a = foo (-__INT_MAX__ - 1); > >> constexpr int b = bar (-__INT_MAX__ - 1); > >> shows that we don't diagnose the latter though, most likely because > >> constexpr evaluation is done on the folded tree. > > I don't think there's any folding before constexpr evaluation in this > case, since this doesn't involve a call. > > >> So, either whatever cp_fold does (which uses fold* underneath) should avoid > >> folding such cases, or the constexpr evaluation should be done on a copy > >> made before folding. Then cp_fold doesn't have to prohibit optimizations > >> and it is a matter of constexpr.c routines to detect all the undefined > >> behavior. After all, I think doing constexpr evaluation on unfolded trees > >> will have also the advantage of better diagnostic locations. > > > > Yes, I think constexpr diagnostic / detection should be done on > > unfolded trees (not sure why we'd need to a copy here, just do folding > > later?). If cp_fold already avoids folding 0 * (-x) then it should > > simply avoid folding 0 / x or 0 % x as well (for the particular issue > > this thread started on). > > The issue is with constexpr functions, which get delayed folding like > any other function before they are used in constant expression > evaluation. The copy would be to preserve the unfolded trees through > cp_fold_function. I've been thinking about doing this anyway; this > may be the motivation to go ahead with it. Or delay the folding until genericization - that is, after all parsing is complete. Not sure what this would do to memory usage or compile-time when we keep all unfolded bodies (or even re-use them in template instantiation). Richard. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-10 7:11 ` Richard Biener @ 2016-06-10 16:16 ` Jason Merrill 2016-06-10 18:56 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Jason Merrill @ 2016-06-10 16:16 UTC (permalink / raw) To: Richard Biener; +Cc: Jakub Jelinek, gcc-patches List On Fri, Jun 10, 2016 at 3:11 AM, Richard Biener <rguenther@suse.de> wrote: > On Thu, 9 Jun 2016, Jason Merrill wrote: > >> On Thu, Jun 9, 2016 at 3:39 AM, Richard Biener <rguenther@suse.de> wrote: >> > On Thu, 9 Jun 2016, Jakub Jelinek wrote: >> > >> >> On Thu, Jun 09, 2016 at 08:50:15AM +0200, Richard Biener wrote: >> >> > On Wed, 8 Jun 2016, Jason Merrill wrote: >> >> > >> >> > > On Wed, Jun 8, 2016 at 11:16 AM, Marc Glisse <marc.glisse@inria.fr> wrote: >> >> > > > On Wed, 8 Jun 2016, Richard Biener wrote: >> >> > > > >> >> > > >> The following works around PR70992 but the issue came up repeatedly >> >> > > >> that we are not very consistent in preserving the undefined behavior >> >> > > >> of division or modulo by zero. Ok - the only inconsistency is >> >> > > >> that we fold 0 % x to 0 but not 0 % 0 (with literal zero). >> >> > > >> >> >> > > >> After folding is now no longer done early in the C family FEs the >> >> > > >> number of diagnostic regressions with the patch below is two. >> >> > > >> >> >> > > >> FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test for excess errors) >> >> > > >> >> > > Yep. We don't want to fold away undefined behavior in a constexpr >> >> > > function, since constexpr evaluation wants to detect undefined >> >> > > behavior and treat the expression as non-constant in that case. >> >> > >> >> > Hmm. So 0 / x is not constant because x might be zero but 0 * x is >> >> > constant because it can never invoke undefined behavior? Does this mean >> >> > that 0 << n is not const because n might be too large (I suppose >> >> > 0 << 12000 is not const already)? Is 0 * (-x) const? x might be INT_MIN. >> >> >> >> E.g. for the shifts the C++ FE has cxx_eval_check_shift_p which should >> >> optionally warn and/or set *non_constant_p. 0 * (-INT_MIN) would be >> >> non-constant too, etc. >> >> constexpr int foo (int x) { return -x; } >> >> constexpr int bar (int x) { return 0 * (-x); } >> >> constexpr int a = foo (-__INT_MAX__ - 1); >> >> constexpr int b = bar (-__INT_MAX__ - 1); >> >> shows that we don't diagnose the latter though, most likely because >> >> constexpr evaluation is done on the folded tree. >> >> I don't think there's any folding before constexpr evaluation in this >> case, since this doesn't involve a call. >> >> >> So, either whatever cp_fold does (which uses fold* underneath) should avoid >> >> folding such cases, or the constexpr evaluation should be done on a copy >> >> made before folding. Then cp_fold doesn't have to prohibit optimizations >> >> and it is a matter of constexpr.c routines to detect all the undefined >> >> behavior. After all, I think doing constexpr evaluation on unfolded trees >> >> will have also the advantage of better diagnostic locations. >> > >> > Yes, I think constexpr diagnostic / detection should be done on >> > unfolded trees (not sure why we'd need to a copy here, just do folding >> > later?). If cp_fold already avoids folding 0 * (-x) then it should >> > simply avoid folding 0 / x or 0 % x as well (for the particular issue >> > this thread started on). >> >> The issue is with constexpr functions, which get delayed folding like >> any other function before they are used in constant expression >> evaluation. The copy would be to preserve the unfolded trees through >> cp_fold_function. I've been thinking about doing this anyway; this >> may be the motivation to go ahead with it. > > Or delay the folding until genericization - that is, after all parsing > is complete. Not sure what this would do to memory usage or compile-time > when we keep all unfolded bodies (or even re-use them in template > instantiation). Genericization happens at the end of each (non-template) function, not at EOF. Do you mean delay until gimplification? Jason ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently 2016-06-10 16:16 ` Jason Merrill @ 2016-06-10 18:56 ` Richard Biener 0 siblings, 0 replies; 13+ messages in thread From: Richard Biener @ 2016-06-10 18:56 UTC (permalink / raw) To: Jason Merrill; +Cc: Jakub Jelinek, gcc-patches List On June 10, 2016 6:15:25 PM GMT+02:00, Jason Merrill <jason@redhat.com> wrote: >On Fri, Jun 10, 2016 at 3:11 AM, Richard Biener <rguenther@suse.de> >wrote: >> On Thu, 9 Jun 2016, Jason Merrill wrote: >> >>> On Thu, Jun 9, 2016 at 3:39 AM, Richard Biener <rguenther@suse.de> >wrote: >>> > On Thu, 9 Jun 2016, Jakub Jelinek wrote: >>> > >>> >> On Thu, Jun 09, 2016 at 08:50:15AM +0200, Richard Biener wrote: >>> >> > On Wed, 8 Jun 2016, Jason Merrill wrote: >>> >> > >>> >> > > On Wed, Jun 8, 2016 at 11:16 AM, Marc Glisse ><marc.glisse@inria.fr> wrote: >>> >> > > > On Wed, 8 Jun 2016, Richard Biener wrote: >>> >> > > > >>> >> > > >> The following works around PR70992 but the issue came up >repeatedly >>> >> > > >> that we are not very consistent in preserving the >undefined behavior >>> >> > > >> of division or modulo by zero. Ok - the only >inconsistency is >>> >> > > >> that we fold 0 % x to 0 but not 0 % 0 (with literal zero). >>> >> > > >> >>> >> > > >> After folding is now no longer done early in the C family >FEs the >>> >> > > >> number of diagnostic regressions with the patch below is >two. >>> >> > > >> >>> >> > > >> FAIL: g++.dg/cpp1y/constexpr-sfinae.C -std=c++14 (test >for excess errors) >>> >> > > >>> >> > > Yep. We don't want to fold away undefined behavior in a >constexpr >>> >> > > function, since constexpr evaluation wants to detect >undefined >>> >> > > behavior and treat the expression as non-constant in that >case. >>> >> > >>> >> > Hmm. So 0 / x is not constant because x might be zero but 0 * >x is >>> >> > constant because it can never invoke undefined behavior? Does >this mean >>> >> > that 0 << n is not const because n might be too large (I >suppose >>> >> > 0 << 12000 is not const already)? Is 0 * (-x) const? x might >be INT_MIN. >>> >> >>> >> E.g. for the shifts the C++ FE has cxx_eval_check_shift_p which >should >>> >> optionally warn and/or set *non_constant_p. 0 * (-INT_MIN) would >be >>> >> non-constant too, etc. >>> >> constexpr int foo (int x) { return -x; } >>> >> constexpr int bar (int x) { return 0 * (-x); } >>> >> constexpr int a = foo (-__INT_MAX__ - 1); >>> >> constexpr int b = bar (-__INT_MAX__ - 1); >>> >> shows that we don't diagnose the latter though, most likely >because >>> >> constexpr evaluation is done on the folded tree. >>> >>> I don't think there's any folding before constexpr evaluation in >this >>> case, since this doesn't involve a call. >>> >>> >> So, either whatever cp_fold does (which uses fold* underneath) >should avoid >>> >> folding such cases, or the constexpr evaluation should be done on >a copy >>> >> made before folding. Then cp_fold doesn't have to prohibit >optimizations >>> >> and it is a matter of constexpr.c routines to detect all the >undefined >>> >> behavior. After all, I think doing constexpr evaluation on >unfolded trees >>> >> will have also the advantage of better diagnostic locations. >>> > >>> > Yes, I think constexpr diagnostic / detection should be done on >>> > unfolded trees (not sure why we'd need to a copy here, just do >folding >>> > later?). If cp_fold already avoids folding 0 * (-x) then it >should >>> > simply avoid folding 0 / x or 0 % x as well (for the particular >issue >>> > this thread started on). >>> >>> The issue is with constexpr functions, which get delayed folding >like >>> any other function before they are used in constant expression >>> evaluation. The copy would be to preserve the unfolded trees >through >>> cp_fold_function. I've been thinking about doing this anyway; this >>> may be the motivation to go ahead with it. >> >> Or delay the folding until genericization - that is, after all >parsing >> is complete. Not sure what this would do to memory usage or >compile-time >> when we keep all unfolded bodies (or even re-use them in template >> instantiation). > >Genericization happens at the end of each (non-template) function, not >at EOF. Do you mean delay until gimplification? Delay until after parsing, yes. Richard. >Jason ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-06-10 18:56 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-08 10:42 [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently Richard Biener 2016-06-08 15:16 ` Marc Glisse 2016-06-08 17:44 ` Jason Merrill 2016-06-08 17:52 ` Jakub Jelinek 2016-06-08 22:18 ` Marc Glisse 2016-06-09 6:53 ` Richard Biener 2016-06-09 6:50 ` Richard Biener 2016-06-09 7:16 ` Jakub Jelinek 2016-06-09 7:39 ` Richard Biener 2016-06-09 22:07 ` Jason Merrill 2016-06-10 7:11 ` Richard Biener 2016-06-10 16:16 ` Jason Merrill 2016-06-10 18:56 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).