[PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Richard Biener]

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)

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Marc Glisse]

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

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Jason Merrill]

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

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Jakub Jelinek]

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

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Marc Glisse]

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

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Richard Biener]

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.

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Richard Biener]

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.

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Jakub Jelinek]

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

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Richard Biener]

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.

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Jason Merrill]

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

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Richard Biener]

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.

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Jason Merrill]

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

Re: [PATCH] Fold x/x to 1, 0/x to 0 and 0%x to 0 consistently

[Richard Biener]

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