public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases
@ 2022-02-02 22:56 pinskia at gcc dot gnu.org
  2022-02-02 22:57 ` [Bug tree-optimization/104356] " pinskia at gcc dot gnu.org
                   ` (47 more replies)
  0 siblings, 48 replies; 49+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-02 22:56 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

            Bug ID: 104356
           Summary: [12 Regression] divide by zero trap is being removed
                    now when it should not be in some cases
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
  Target Milestone: ---

Created attachment 52335
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52335&action=edit
gnat.dg/div_zero.adb

>From https://gcc.gnu.org/pipermail/gcc-patches/2022-January/589490.html

Right, for example in Ada where we now happily turn a division by zero, which 
should raise an exception with -gnatp, into nonsense.  Do we really need this 
rather useless optimization in GCC?  Blindly mimicing LLVM is not a reason...

I have installed the attached testcase, which now fails because of the change.

        * gnat.dg/div_zero.adb: New test.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
@ 2022-02-02 22:57 ` pinskia at gcc dot gnu.org
  2022-02-02 23:08 ` jakub at gcc dot gnu.org
                   ` (46 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-02 22:57 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
  2022-02-02 22:57 ` [Bug tree-optimization/104356] " pinskia at gcc dot gnu.org
@ 2022-02-02 23:08 ` jakub at gcc dot gnu.org
  2022-02-03  7:32 ` rguenth at gcc dot gnu.org
                   ` (45 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-02 23:08 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So let's guard the optimization on !flag_non_call_exceptions or so?
The libgcc2.c
          if (d0 == 0)
            d0 = 1 / d0;        /* Divide intentionally by zero.  */
cases IMHO just should hide it from the optimizers.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
  2022-02-02 22:57 ` [Bug tree-optimization/104356] " pinskia at gcc dot gnu.org
  2022-02-02 23:08 ` jakub at gcc dot gnu.org
@ 2022-02-03  7:32 ` rguenth at gcc dot gnu.org
  2022-02-03  7:42 ` pinskia at gcc dot gnu.org
                   ` (44 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-03  7:32 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, but doesn't ada enable -fdelete-dead-exceptions?  That is, I'm not sure
we make division by zero well-defined with -fnon-call-exceptions - the
transform assumes the exception cannot happen (because undefinedness) and
removes the exceptional path.

So do we, with -fnon-call-exceptions, make division by zero throw an exception
instead of invoking undefined behavior?  (that is, on the non-exceptional
path we can still assume the divisor is not zero after the division)

Our docs say

@item -fnon-call-exceptions
@opindex fnon-call-exceptions
Generate code that allows trapping instructions to throw exceptions.
Note that this requires platform-specific runtime support that does
not exist everywhere.  Moreover, it only allows @emph{trapping}
instructions to throw exceptions, i.e.@: memory references or floating-point
instructions.  It does not allow exceptions to be thrown from
arbitrary signal handlers such as @code{SIGALRM}.  This enables
@option{-fexceptions}.

which doesn't mention integer division by zero or arbitrary instructions
of the ISA that may raise an exception under some circumstances (IIRC some
x86 ones raise exceptions on overflow for example).  Like the C standard
would allow the CPU to raise an exception on out-of-bound shifts (since
those invoke undefined behavior) - if the CPU would, would
-fnon-call-exceptions
imply it's now implementation defined (throw an exception?)?


Of course integer division might be special enough to single out as IIRC
all CPUs trap on that (I'm sure somebody knows one that does not...).

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2022-02-03  7:32 ` rguenth at gcc dot gnu.org
@ 2022-02-03  7:42 ` pinskia at gcc dot gnu.org
  2022-02-03  7:43 ` pinskia at gcc dot gnu.org
                   ` (43 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-03  7:42 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> Of course integer division might be special enough to single out as IIRC
> all CPUs trap on that (I'm sure somebody knows one that does not...).

MIPS backend adds the trap (it can be turned off by a backend flag).
AARCH64 and PowerPC don't trap either.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2022-02-03  7:42 ` pinskia at gcc dot gnu.org
@ 2022-02-03  7:43 ` pinskia at gcc dot gnu.org
  2022-02-03  7:58 ` pinskia at gcc dot gnu.org
                   ` (42 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-03  7:43 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> (In reply to Richard Biener from comment #2)
> > Of course integer division might be special enough to single out as IIRC
> > all CPUs trap on that (I'm sure somebody knows one that does not...).
> 
> MIPS backend adds the trap (it can be turned off by a backend flag).
> AARCH64 and PowerPC don't trap either.

The Java front-end used to have option which caused the front-end to emit the
throw/trap for the division by zero which was turned off for a few targets (I
can't remember if it was a hook or hard coded).

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2022-02-03  7:43 ` pinskia at gcc dot gnu.org
@ 2022-02-03  7:58 ` pinskia at gcc dot gnu.org
  2022-02-03  8:34 ` rguenther at suse dot de
                   ` (41 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-03  7:58 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #4)
> The Java front-end used to have option which caused the front-end to emit
> the throw/trap for the division by zero which was turned off for a few
> targets (I can't remember if it was a hook or hard coded).

So I looked and it looks like I was wrong about the (old) java front-end, it
defaulted to always calling the function to do the divison and just throw in
those functions.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2022-02-03  7:58 ` pinskia at gcc dot gnu.org
@ 2022-02-03  8:34 ` rguenther at suse dot de
  2022-02-03 10:40 ` ebotcazou at gcc dot gnu.org
                   ` (40 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenther at suse dot de @ 2022-02-03  8:34 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 3 Feb 2022, pinskia at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356
> 
> --- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> (In reply to Andrew Pinski from comment #3)
> > (In reply to Richard Biener from comment #2)
> > > Of course integer division might be special enough to single out as IIRC
> > > all CPUs trap on that (I'm sure somebody knows one that does not...).
> > 
> > MIPS backend adds the trap (it can be turned off by a backend flag).
> > AARCH64 and PowerPC don't trap either.
> 
> The Java front-end used to have option which caused the front-end to emit the
> throw/trap for the division by zero which was turned off for a few targets (I
> can't remember if it was a hook or hard coded).

I suppose a set of [us]div_trap_on_zero patterns could be used to
merge a conditional trap with division during combine if we want to go
there.  But that suggests that if in Ada integer divide by zero is
something the language standard mandates special behavior for then
either the frontend (with help from the middle-end) or the
middle-end (by singling out integer division with -fnon-call-exceptions)
needs to make the operation well-defined - preferably in an explicit way.
These days we'd probably add IFN_TRUNC_DIV_OR_TRAP for this purpose.

I don't mind guarding the pattern for GCC 12 though, but I'd like
to clarify the semantic questions raised here.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2022-02-03  8:34 ` rguenther at suse dot de
@ 2022-02-03 10:40 ` ebotcazou at gcc dot gnu.org
  2022-02-03 11:08 ` rguenther at suse dot de
                   ` (39 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 10:40 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-02-03
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Hmm, but doesn't ada enable -fdelete-dead-exceptions?  That is, I'm not sure
> we make division by zero well-defined with -fnon-call-exceptions - the
> transform assumes the exception cannot happen (because undefinedness) and
> removes the exceptional path.

Yes, the division by zero can be optimized away in Ada if the result of the
operation is not used later, so we would need to add a pragma Volatile to the
gnat.dg/div_zero.adb testcase:

  D : Integer := Zero;
  pragma Volatile (D);

for strict semantics, but this usually does not matter at -O0 when the result
is assigned to a user variable:

  D := 1 / D;

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2022-02-03 10:40 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 11:08 ` rguenther at suse dot de
  2022-02-03 11:14 ` ebotcazou at gcc dot gnu.org
                   ` (38 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenther at suse dot de @ 2022-02-03 11:08 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #8 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 3 Feb 2022, ebotcazou at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356
> 
> Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>              Status|UNCONFIRMED                 |NEW
>      Ever confirmed|0                           |1
>    Last reconfirmed|                            |2022-02-03
>                  CC|                            |ebotcazou at gcc dot gnu.org
> 
> --- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> > Hmm, but doesn't ada enable -fdelete-dead-exceptions?  That is, I'm not sure
> > we make division by zero well-defined with -fnon-call-exceptions - the
> > transform assumes the exception cannot happen (because undefinedness) and
> > removes the exceptional path.
> 
> Yes, the division by zero can be optimized away in Ada if the result of the
> operation is not used later, so we would need to add a pragma Volatile to the
> gnat.dg/div_zero.adb testcase:
> 
>   D : Integer := Zero;
>   pragma Volatile (D);
> 
> for strict semantics, but this usually does not matter at -O0 when the result
> is assigned to a user variable:
> 
>   D := 1 / D;

So for Ada it would be valid to optimize it as

  tem = D;
  if (tem != 0)
    D := 1 / tem;
  else
    D = tem;

basically carrying out the division conditionally only?
(I've tried hard to preserve all volatile loads / stores, if not
volatile that can be elided)

Does Ada define what value D obtains when D is zero or does it
only allow the divison and the exceptional case to be optimized
together but not separate?  So optimization to

  tem = D;
  if (tem != 0)
    D := 1 / tem;
  else
    1/0;

and then optimizing the unused 1/0 "exceptional case" only is not
allowed?

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2022-02-03 11:08 ` rguenther at suse dot de
@ 2022-02-03 11:14 ` ebotcazou at gcc dot gnu.org
  2022-02-03 11:22 ` jakub at gcc dot gnu.org
                   ` (37 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 11:14 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> So for Ada it would be valid to optimize it as
> 
>   tem = D;
>   if (tem != 0)
>     D := 1 / tem;
>   else
>     D = tem;
> 
> basically carrying out the division conditionally only?
> (I've tried hard to preserve all volatile loads / stores, if not
> volatile that can be elided)

No, the result of the operation is not dead here (modulo Volatile), so the
operation must be performed and therefore raise an exception.  To sum up,
either you optimize away the final assignment to D or you raise an exception,
i.e. D cannot contain a random value.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2022-02-03 11:14 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 11:22 ` jakub at gcc dot gnu.org
  2022-02-03 11:41 ` ebotcazou at gcc dot gnu.org
                   ` (36 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 11:22 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So what does Ada do on targets like powerpc that do not raise an exception?
>From what I can see, 1 / 0 yields 0 there.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2022-02-03 11:22 ` jakub at gcc dot gnu.org
@ 2022-02-03 11:41 ` ebotcazou at gcc dot gnu.org
  2022-02-03 11:43 ` rguenth at gcc dot gnu.org
                   ` (35 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 11:41 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #11 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> So what does Ada do on targets like powerpc that do not raise an exception?
> From what I can see, 1 / 0 yields 0 there.

It generates an explicit check for division-by-zero in the general case (remove
the pragma Suppress (All_Checks) and you'll see it) but we do not rely on it
everywhere, only where system.ads contains:

   Backend_Divide_Checks : constant Boolean := False;

In other words, when Backend_Divide_Checks is set to True, we need the divide
and it needs to trap (and pragma Suppress (All_Checks) emulates that, but I
certainly need to add a dg-skip for PowerPC here).

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2022-02-03 11:41 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 11:43 ` rguenth at gcc dot gnu.org
  2022-02-03 11:45 ` jakub at gcc dot gnu.org
                   ` (34 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-03 11:43 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #9)
> > So for Ada it would be valid to optimize it as
> > 
> >   tem = D;
> >   if (tem != 0)
> >     D := 1 / tem;
> >   else
> >     D = tem;
> > 
> > basically carrying out the division conditionally only?
> > (I've tried hard to preserve all volatile loads / stores, if not
> > volatile that can be elided)
> 
> No, the result of the operation is not dead here (modulo Volatile), so the
> operation must be performed and therefore raise an exception.  To sum up,
> either you optimize away the final assignment to D or you raise an
> exception, i.e. D cannot contain a random value.

OK, so a division by zero is not invoking undefined behavior but is
well-defined and traps.  And the idea is that -fnon-call-exceptions alone
carries this over to middle-end semantics?  (I don't think it does that at the
moment, with or without the rev. in question)

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2022-02-03 11:43 ` rguenth at gcc dot gnu.org
@ 2022-02-03 11:45 ` jakub at gcc dot gnu.org
  2022-02-03 11:48 ` ebotcazou at gcc dot gnu.org
                   ` (33 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 11:45 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Anyway, if integer division by zero is not considered UB in Ada, we need some
flag_whatever that will represent that to the middle-end and check it in all
places where we assume it is UB.  It can be of course set e.g. only if the
explicit checks aren't emitted.
And this optimization would be just one of the many places that would need to
be guarded by that flag.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2022-02-03 11:45 ` jakub at gcc dot gnu.org
@ 2022-02-03 11:48 ` ebotcazou at gcc dot gnu.org
  2022-02-03 11:50 ` ebotcazou at gcc dot gnu.org
                   ` (32 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 11:48 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #14 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> OK, so a division by zero is not invoking undefined behavior but is
> well-defined and traps.  And the idea is that -fnon-call-exceptions alone
> carries this over to middle-end semantics?  (I don't think it does that at
> the moment, with or without the rev. in question)

Why?  It's exactly like any other trapping operation, see the numerous
predicates in tree-eh.cc, most notably operation_could_trap_helper_p:

  switch (op)
    {
    case TRUNC_DIV_EXPR:
    case CEIL_DIV_EXPR:
    case FLOOR_DIV_EXPR:
    case ROUND_DIV_EXPR:
    case EXACT_DIV_EXPR:
    case CEIL_MOD_EXPR:
    case FLOOR_MOD_EXPR:
    case ROUND_MOD_EXPR:
    case TRUNC_MOD_EXPR:
      if (!TREE_CONSTANT (divisor) || integer_zerop (divisor))
        return true;

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2022-02-03 11:48 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 11:50 ` ebotcazou at gcc dot gnu.org
  2022-02-03 11:52 ` rguenth at gcc dot gnu.org
                   ` (31 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 11:50 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #15 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
And in rtlanal.cc, see may_trap_p_1:

      /* Division by a non-constant might trap.  */
    case DIV:
    case MOD:
    case UDIV:
    case UMOD:
      if (HONOR_SNANS (x))
        return 1;
      if (FLOAT_MODE_P (GET_MODE (x)))
        return flag_trapping_math;
      if (!CONSTANT_P (XEXP (x, 1)) || (XEXP (x, 1) == const0_rtx))
        return 1;

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (15 preceding siblings ...)
  2022-02-03 11:50 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 11:52 ` rguenth at gcc dot gnu.org
  2022-02-03 11:56 ` rguenth at gcc dot gnu.org
                   ` (30 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-03 11:52 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Related simplifications:

 /* X / bool_range_Y is X.  */
 (simplify
  (div @0 SSA_NAME@1)
  (if (INTEGRAL_TYPE_P (type) && ssa_name_has_boolean_range (@1))
   @0))

 /* X / abs (X) is X < 0 ? -1 : 1.  */
 (simplify
   (div:C @0 (abs @0))
   (if (INTEGRAL_TYPE_P (type)
        && TYPE_OVERFLOW_UNDEFINED (type))
    (cond (lt @0 { build_zero_cst (type); })
          { build_minus_one_cst (type); } { build_one_cst (type); })))

 /* X / -X is -1.  */
 (simplify
   (div:C @0 (negate @0))
   (if ((INTEGRAL_TYPE_P (type) || VECTOR_INTEGER_TYPE_P (type))
        && TYPE_OVERFLOW_UNDEFINED (type))
    { build_minus_one_cst (type); })))

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (16 preceding siblings ...)
  2022-02-03 11:52 ` rguenth at gcc dot gnu.org
@ 2022-02-03 11:56 ` rguenth at gcc dot gnu.org
  2022-02-03 12:00 ` rguenth at gcc dot gnu.org
                   ` (29 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-03 11:56 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #14)
> > OK, so a division by zero is not invoking undefined behavior but is
> > well-defined and traps.  And the idea is that -fnon-call-exceptions alone
> > carries this over to middle-end semantics?  (I don't think it does that at
> > the moment, with or without the rev. in question)
> 
> Why?  It's exactly like any other trapping operation, see the numerous
> predicates in tree-eh.cc, most notably operation_could_trap_helper_p:
> 
>   switch (op)
>     {
>     case TRUNC_DIV_EXPR:
>     case CEIL_DIV_EXPR:
>     case FLOOR_DIV_EXPR:
>     case ROUND_DIV_EXPR:
>     case EXACT_DIV_EXPR:
>     case CEIL_MOD_EXPR:
>     case FLOOR_MOD_EXPR:
>     case ROUND_MOD_EXPR:
>     case TRUNC_MOD_EXPR:
>       if (!TREE_CONSTANT (divisor) || integer_zerop (divisor))
>         return true;

It's used for a different purpose as well, we of course have to avoid moving a
possibly trapping operation to a place where it was not executed
unconditionally.

But sure, so you say that -fnon-call-exceptions makes operations that
may trap according to the EH machinery well-defined, irrespective of what
the language standards say?  That certainly makes sense, it doesn't
make sense to create EH when the situation invokes undefined behavior.
(but as said above we have to avoid traps in non-EH context as well)

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (17 preceding siblings ...)
  2022-02-03 11:56 ` rguenth at gcc dot gnu.org
@ 2022-02-03 12:00 ` rguenth at gcc dot gnu.org
  2022-02-03 12:00 ` jakub at gcc dot gnu.org
                   ` (28 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-03 12:00 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
So we'd add

  (!flag_non_call_exceptions || tree_expr_nonzero_p (@..))

to the offending and extra listed match.pd transforms.  I guess that works for
me.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (18 preceding siblings ...)
  2022-02-03 12:00 ` rguenth at gcc dot gnu.org
@ 2022-02-03 12:00 ` jakub at gcc dot gnu.org
  2022-02-03 12:03 ` jakub at gcc dot gnu.org
                   ` (27 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 12:00 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Those correctly say that division by zero may trap.  That doesn't tell that
division by zero is not undefined behavior, but well defined to trap.
This new optimization, or e.g. operator_div::wi_fold's
  // If we're definitely dividing by zero, there's nothing to do.
  if (wi_zero_p (type, divisor_min, divisor_max))
    {
      r.set_undefined ();
      return;
    }
but even that
  // Perform the division in 2 parts, [LB, -1] and [1, UB], which will
  // skip any division by zero.
following it, I'm afraid a few further spots in match.pd, e.g.
 /* X / bool_range_Y is X.  */
 (simplify
  (div @0 SSA_NAME@1)
  (if (INTEGRAL_TYPE_P (type) && ssa_name_has_boolean_range (@1))
   @0))
or
 /* X / X is one.  */
 (simplify
  (div @0 @0)
  /* But not for 0 / 0 so that we can get the proper warnings and errors.
     And not for _Fract types where we can't build 1.  */
  (if (!integer_zerop (@0) && !ALL_FRACT_MODE_P (TYPE_MODE (type)))
   { build_one_cst (type); }))
 /* X / abs (X) is X < 0 ? -1 : 1.  */
 (simplify
   (div:C @0 (abs @0))
   (if (INTEGRAL_TYPE_P (type)
        && TYPE_OVERFLOW_UNDEFINED (type))
    (cond (lt @0 { build_zero_cst (type); })
          { build_minus_one_cst (type); } { build_one_cst (type); })))
 /* X / -X is -1.  */
 (simplify
   (div:C @0 (negate @0))
   (if ((INTEGRAL_TYPE_P (type) || VECTOR_INTEGER_TYPE_P (type))
        && TYPE_OVERFLOW_UNDEFINED (type))
    { build_minus_one_cst (type); })))
etc.
I'm afraid it is all over the middle-end.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (19 preceding siblings ...)
  2022-02-03 12:00 ` jakub at gcc dot gnu.org
@ 2022-02-03 12:03 ` jakub at gcc dot gnu.org
  2022-02-03 12:03 ` ebotcazou at gcc dot gnu.org
                   ` (26 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 12:03 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
flag_non_call_exceptions can be used even in C/C++ and other languages, so I'd
strongly prefer a new flag which will e.g. make it clearer what is it about,
will make it easier for grep for such dependencies etc.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (20 preceding siblings ...)
  2022-02-03 12:03 ` jakub at gcc dot gnu.org
@ 2022-02-03 12:03 ` ebotcazou at gcc dot gnu.org
  2022-02-03 12:05 ` ebotcazou at gcc dot gnu.org
                   ` (25 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 12:03 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #21 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> But sure, so you say that -fnon-call-exceptions makes operations that
> may trap according to the EH machinery well-defined, irrespective of what
> the language standards say?  That certainly makes sense, it doesn't
> make sense to create EH when the situation invokes undefined behavior.
> (but as said above we have to avoid traps in non-EH context as well)

Of course, that's the very and only purpose of -fnon-call-exceptions, see
tree_could_throw_p:

/* Return true if expression T could throw an exception.  */

bool
tree_could_throw_p (tree t)
{
[...]
  if (cfun->can_throw_non_call_exceptions)
    return tree_could_trap_p (t);
  return false;
}

With -fnon-call-exceptions, trapping operations are not UB but throw instead.
What the language (Java, Go, Ada) makes of this exception may vary, but I don't
think that they call it undefined behavior like in the C family of languages.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (21 preceding siblings ...)
  2022-02-03 12:03 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 12:05 ` ebotcazou at gcc dot gnu.org
  2022-02-03 12:05 ` rguenther at suse dot de
                   ` (24 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 12:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #22 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> So we'd add
> 
>   (!flag_non_call_exceptions || tree_expr_nonzero_p (@..))
> 
> to the offending and extra listed match.pd transforms.  I guess that works
> for me.

For me as well.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (22 preceding siblings ...)
  2022-02-03 12:05 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 12:05 ` rguenther at suse dot de
  2022-02-03 12:10 ` rguenth at gcc dot gnu.org
                   ` (23 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenther at suse dot de @ 2022-02-03 12:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #23 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 3 Feb 2022, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356
> 
> --- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> flag_non_call_exceptions can be used even in C/C++ and other languages, so I'd
> strongly prefer a new flag which will e.g. make it clearer what is it about,
> will make it easier for grep for such dependencies etc.

But then it doesn't make sense to create EH edges from integer divisions
when the only exceptional case is invoking undefined behavior.

I think we need to align what is undefined and what is exceptional,
a new flag only makes the situation worse.

The option besides making -fnon-call-exceptions do it would be to
require the frontend to emit different GENERIC, aka some
.DIV_WITH_EH_ON_ZERO or however we want to represent that.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (23 preceding siblings ...)
  2022-02-03 12:05 ` rguenther at suse dot de
@ 2022-02-03 12:10 ` rguenth at gcc dot gnu.org
  2022-02-03 12:13 ` jakub at gcc dot gnu.org
                   ` (22 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-03 12:10 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #24 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #22)
> > So we'd add
> > 
> >   (!flag_non_call_exceptions || tree_expr_nonzero_p (@..))
> > 
> > to the offending and extra listed match.pd transforms.  I guess that works
> > for me.
> 
> For me as well.

Oh, with the little detail that it should be
cfun->can_throw_non_call_exceptions
(with the unfortunate reference to cfun)

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (24 preceding siblings ...)
  2022-02-03 12:10 ` rguenth at gcc dot gnu.org
@ 2022-02-03 12:13 ` jakub at gcc dot gnu.org
  2022-02-03 12:18 ` ebotcazou at gcc dot gnu.org
                   ` (21 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 12:13 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #25 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GENERIC folding can be done with NULL cfun too though.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (25 preceding siblings ...)
  2022-02-03 12:13 ` jakub at gcc dot gnu.org
@ 2022-02-03 12:18 ` ebotcazou at gcc dot gnu.org
  2022-02-03 12:21 ` cvs-commit at gcc dot gnu.org
                   ` (20 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 12:18 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #26 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Oh, with the little detail that it should be
> cfun->can_throw_non_call_exceptions
> (with the unfortunate reference to cfun)

Yes, and note that this will also solve the libgcc2.c issue because of:

ifeq ($(LIB2_DIVMOD_EXCEPTION_FLAGS),)
# Provide default flags for compiling divmod functions, if they haven't been
# set already by a target-specific Makefile fragment.
LIB2_DIVMOD_EXCEPTION_FLAGS := -fexceptions -fnon-call-exceptions
endif

# Build LIB2_DIVMOD_FUNCS.
lib2-divmod-o = $(patsubst %,%$(objext),$(LIB2_DIVMOD_FUNCS))
$(lib2-divmod-o): %$(objext): $(srcdir)/libgcc2.c
        $(gcc_compile) -DL$* -c $< \
          $(LIB2_DIVMOD_EXCEPTION_FLAGS) $(vis_hide)
libgcc-objects += $(lib2-divmod-o)

ifeq ($(enable_shared),yes)
lib2-divmod-s-o = $(patsubst %,%_s$(objext),$(LIB2_DIVMOD_FUNCS))
$(lib2-divmod-s-o): %_s$(objext): $(srcdir)/libgcc2.c
        $(gcc_s_compile) -DL$* -c $< \
          $(LIB2_DIVMOD_EXCEPTION_FLAGS)
libgcc-s-objects += $(lib2-divmod-s-o)
endif

in libgcc/Makefile.in.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (26 preceding siblings ...)
  2022-02-03 12:18 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 12:21 ` cvs-commit at gcc dot gnu.org
  2022-02-03 12:28 ` ebotcazou at gcc dot gnu.org
                   ` (19 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-02-03 12:21 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #27 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Eric Botcazou <ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:635504510a9410844991c68880f2e7352cacfd86

commit r12-7021-g635504510a9410844991c68880f2e7352cacfd86
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Thu Feb 3 13:12:37 2022 +0100

    Skip gnat.dg/div_zero.adb on PowerPC

    The hardware instruction does not trap on divide by zero there.

    gcc/testsuite
            PR tree-optimization/104356
            * gnat.dg/div_zero.adb: Add dg-skip-if directive for PowerPC.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (27 preceding siblings ...)
  2022-02-03 12:21 ` cvs-commit at gcc dot gnu.org
@ 2022-02-03 12:28 ` ebotcazou at gcc dot gnu.org
  2022-02-03 12:39 ` jakub at gcc dot gnu.org
                   ` (18 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 12:28 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #28 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Oh, with the little detail that it should be
> cfun->can_throw_non_call_exceptions
> (with the unfortunate reference to cfun)

We have global references to flag_trapping_math in match.pd so I think that
global references to flag_non_call_exceptions can do too.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (28 preceding siblings ...)
  2022-02-03 12:28 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 12:39 ` jakub at gcc dot gnu.org
  2022-02-03 12:49 ` ebotcazou at gcc dot gnu.org
                   ` (17 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 12:39 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #29 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #24)
> (In reply to Eric Botcazou from comment #22)
> > > So we'd add
> > > 
> > >   (!flag_non_call_exceptions || tree_expr_nonzero_p (@..))
> > > 
> > > to the offending and extra listed match.pd transforms.  I guess that works
> > > for me.
> > 
> > For me as well.
> 
> Oh, with the little detail that it should be
> cfun->can_throw_non_call_exceptions
> (with the unfortunate reference to cfun)

Why does cfun->can_throw_non_call_exceptions and
cfun->can_delete_dead_exceptions exist btw?  Now that both
flag_non_call_exceptions and flag_delete_dead_exceptions are Optimization,
set_cfun adjusts that on switching the function, so I think it be the same. 
Ok, {fn,fun,src_cfun}->can_throw_non_call_exceptions might be faster when it
looks at other functions than opt_for_fn.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (29 preceding siblings ...)
  2022-02-03 12:39 ` jakub at gcc dot gnu.org
@ 2022-02-03 12:49 ` ebotcazou at gcc dot gnu.org
  2022-02-03 12:51 ` ebotcazou at gcc dot gnu.org
                   ` (16 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 12:49 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #30 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Why does cfun->can_throw_non_call_exceptions and
> cfun->can_delete_dead_exceptions exist btw?

Jan asked me to create them when he was working on option merging for LTO.

> Now that both flag_non_call_exceptions and flag_delete_dead_exceptions are 
> Optimization, set_cfun adjusts that on switching the function, so I think it
> be the same. 

Good news then, thanks!

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (30 preceding siblings ...)
  2022-02-03 12:49 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 12:51 ` ebotcazou at gcc dot gnu.org
  2022-02-03 13:04 ` jakub at gcc dot gnu.org
                   ` (15 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 12:51 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #31 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Created attachment 52340
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52340&action=edit
Tentative fix

To be further tested.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (31 preceding siblings ...)
  2022-02-03 12:51 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 13:04 ` jakub at gcc dot gnu.org
  2022-02-03 13:09 ` rguenther at suse dot de
                   ` (14 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 13:04 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #32 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Comment on attachment 52340
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52340
Tentative fix

For the X / bool_range_Y is X. case I think just !flag_non_call_exceptions
would be better.  If @1 has boolean range and is known to be non-zero, it is
known to be 1, so we should be optimizing it elsewhere, that is the constant
case.

Anyway, that's probably it in match.pd.
path isolation is already guarding is_divmod_with_given_divisor calls with
!cfun->can_throw_non_call_exceptions.
>From the to me known issues that leaves VRP.  As Ada expects the division by
zero to trap, I guess we don't care much about the range it emits for the
result, but it shouldn't be for cfun->can_throw_non_call_exceptions be
undefined or single value range, because then it would be just optimized into a
constant I bet.  But there is also the problem that ranger doesn't walk just
from stmts to its immediate uses, but also from stmts to their operand's def
stmts.
Can you try to rewrite the
unsigned                                                                        
foo (unsigned x, unsigned y)                                                    
{                                                                               
  if (x >= 2)                                                                   
    return 0;                                                                   
  if (x == 1)                                                                   
    y += 4;                                                                     
  return y / x;                                                                 
}                                                                               
testcase I've posted into Ada and see if it will optimize away the division in
evrp or vrp?

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (32 preceding siblings ...)
  2022-02-03 13:04 ` jakub at gcc dot gnu.org
@ 2022-02-03 13:09 ` rguenther at suse dot de
  2022-02-03 15:41 ` ebotcazou at gcc dot gnu.org
                   ` (13 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenther at suse dot de @ 2022-02-03 13:09 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #33 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 3 Feb 2022, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356
> 
> --- Comment #29 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #24)
> > (In reply to Eric Botcazou from comment #22)
> > > > So we'd add
> > > > 
> > > >   (!flag_non_call_exceptions || tree_expr_nonzero_p (@..))
> > > > 
> > > > to the offending and extra listed match.pd transforms.  I guess that works
> > > > for me.
> > > 
> > > For me as well.
> > 
> > Oh, with the little detail that it should be
> > cfun->can_throw_non_call_exceptions
> > (with the unfortunate reference to cfun)
> 
> Why does cfun->can_throw_non_call_exceptions and
> cfun->can_delete_dead_exceptions exist btw?  Now that both
> flag_non_call_exceptions and flag_delete_dead_exceptions are Optimization,
> set_cfun adjusts that on switching the function, so I think it be the same. 
> Ok, {fn,fun,src_cfun}->can_throw_non_call_exceptions might be faster when it
> looks at other functions than opt_for_fn.

I think ->can_throw_non_call_exceptions allows to optimize it based on
IL analysis.  But historically it was because of LTO and before we had
forced the "correct" set of options on all functions.

Btw, !cfun || cfun->... would work as well.

That said, we can probably remove both struct function "copies" of the
flags and replace fun->xxx with OPT_for_fn

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap is being removed now when it should not be in some cases
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (33 preceding siblings ...)
  2022-02-03 13:09 ` rguenther at suse dot de
@ 2022-02-03 15:41 ` ebotcazou at gcc dot gnu.org
  2022-02-03 16:28 ` [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away jakub at gcc dot gnu.org
                   ` (12 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 15:41 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #34 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> For the X / bool_range_Y is X. case I think just !flag_non_call_exceptions
> would be better.  If @1 has boolean range and is known to be non-zero, it is
> known to be 1, so we should be optimizing it elsewhere, that is the constant
> case.

OK, adjusted and successfully tested on x86-64/Linux.

> path isolation is already guarding is_divmod_with_given_divisor calls with
> !cfun->can_throw_non_call_exceptions.

Indeed.

> Can you try to rewrite the
> unsigned                                                                    
> foo (unsigned x, unsigned y)                                                
> {                                                                           
>   if (x >= 2)                                                               
>     return 0;                                                               
>   if (x == 1)                                                               
>     y += 4;                                                                 
>   return y / x;                                                             
> }                                                                           
> 
> testcase I've posted into Ada and see if it will optimize away the division
> in evrp or vrp?

For:

with System.Unsigned_Types; use System.Unsigned_Types;

function F (X, Y : Unsigned) return Unsigned is
begin
  if X >=2 then
    return 0;
  elsif X = 1 then
    return 2 * Y;
  else
    return Y / X;
  end if;
end;

I get with -O2 -gnatp:

_ada_f:
.LFB1:
        .cfi_startproc
        cmpl    $1, %edi
        ja      .L4
        je      .L6
        ud2
        .p2align 4,,10
        .p2align 3
.L4:
        xorl    %eax, %eax
        ret
        .p2align 4,,10
        .p2align 3
.L6:
        leal    (%rsi,%rsi), %eax
        ret

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (34 preceding siblings ...)
  2022-02-03 15:41 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 16:28 ` jakub at gcc dot gnu.org
  2022-02-03 16:41 ` ebotcazou at gcc dot gnu.org
                   ` (11 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 16:28 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #35 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I meant something like:
with System.Unsigned_Types; use System.Unsigned_Types;

function F (X, Y : Unsigned) return Unsigned is
  Z : Unsigned;
begin
  if X >=2 then
    return 0;
  end if;
  Z := Y;
  if X = 1 then
    Z := Y + 4;
  end if;
  return Z / X;
end;
and there evrp does with -O2 -gnatp optimize away the division.
Though that is likely the X / boolean_range_Y case which you've disabled.
In any case, I think you want to hear from Andrew/Aldy where exactly does
VRP/ranger assume UB on integer division by zero.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (35 preceding siblings ...)
  2022-02-03 16:28 ` [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away jakub at gcc dot gnu.org
@ 2022-02-03 16:41 ` ebotcazou at gcc dot gnu.org
  2022-02-03 16:51 ` amacleod at redhat dot com
                   ` (10 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-03 16:41 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #36 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> with System.Unsigned_Types; use System.Unsigned_Types;
> 
> function F (X, Y : Unsigned) return Unsigned is
>   Z : Unsigned;
> begin
>   if X >=2 then
>     return 0;
>   end if;
>   Z := Y;
>   if X = 1 then
>     Z := Y + 4;
>   end if;
>   return Z / X;
> end;
> and there evrp does with -O2 -gnatp optimize away the division.

Indeed, I see this with GCC 11 & 12 (patched or not) but not with GCC 9 & 10.

> Though that is likely the X / boolean_range_Y case which you've disabled.
> In any case, I think you want to hear from Andrew/Aldy where exactly does
> VRP/ranger assume UB on integer division by zero.

Let's close this PR first, which is a 12 regression, and I'll open a PR for the
VRP issue in 11 & 12.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (36 preceding siblings ...)
  2022-02-03 16:41 ` ebotcazou at gcc dot gnu.org
@ 2022-02-03 16:51 ` amacleod at redhat dot com
  2022-02-03 16:57 ` jakub at gcc dot gnu.org
                   ` (9 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: amacleod at redhat dot com @ 2022-02-03 16:51 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #37 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Jakub Jelinek from comment #35)
> I meant something like:
>   return Z / X;

> and there evrp does with -O2 -gnatp optimize away the division.
> Though that is likely the X / boolean_range_Y case which you've disabled.
> In any case, I think you want to hear from Andrew/Aldy where exactly does
> VRP/ranger assume UB on integer division by zero.

That divide is remove by the simplifier because it determines that X has a
range of [0,1] and I believe the simplifer chooses to ignore the 0 under
various circumstances.

As for ranger, range-ops will return UNDEFINED for the range if x is known to
be [0,0].  This can be propagated around, and depending on how it ends up being
used as to what happens with it. 

This happens in range-ops.cc in operator_div::wi_fold()

  // If we're definitely dividing by zero, there's nothing to do.
  if (wi_zero_p (type, divisor_min, divisor_max))
    {
      r.set_undefined ();
      return;
    }

likewise the MOD operator does the same:

void
operator_trunc_mod::wi_fold (irange &r, tree type,
                             const wide_int &lh_lb,
                             const wide_int &lh_ub,
                             const wide_int &rh_lb,
                             const wide_int &rh_ub) const
{
  wide_int new_lb, new_ub, tmp;
  signop sign = TYPE_SIGN (type);
  unsigned prec = TYPE_PRECISION (type);

  // Mod 0 is undefined.
  if (wi_zero_p (type, rh_lb, rh_ub))
    {
      r.set_undefined ();
      return;
    }

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (37 preceding siblings ...)
  2022-02-03 16:51 ` amacleod at redhat dot com
@ 2022-02-03 16:57 ` jakub at gcc dot gnu.org
  2022-02-03 18:35 ` amacleod at redhat dot com
                   ` (8 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-03 16:57 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #38 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Andrew Macleod from comment #37)
> As for ranger, range-ops will return UNDEFINED for the range if x is known
> to be [0,0].  This can be propagated around, and depending on how it ends up
> being used as to what happens with it. 

Yeah, so e.g. with Eric's patch to disable the X / boolean_range_Y simplifier
in match.pd, won't the ranger perform the same optimization?
I mean with the wi_fold_in_parts, if range of the divisor has 2 values and one
of them is 0, won't it try to union range of X / 1 (range of X) and range of X
/ 0 (undefined) and yield range of X?  So say won't 7 / Y_with_bool_range yield
[7,7] ?

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (38 preceding siblings ...)
  2022-02-03 16:57 ` jakub at gcc dot gnu.org
@ 2022-02-03 18:35 ` amacleod at redhat dot com
  2022-02-04  7:08 ` rguenther at suse dot de
                   ` (7 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: amacleod at redhat dot com @ 2022-02-03 18:35 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #39 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Jakub Jelinek from comment #38)
> (In reply to Andrew Macleod from comment #37)
> > As for ranger, range-ops will return UNDEFINED for the range if x is known
> > to be [0,0].  This can be propagated around, and depending on how it ends up
> > being used as to what happens with it. 
> 
> Yeah, so e.g. with Eric's patch to disable the X / boolean_range_Y simplifier
> in match.pd, won't the ranger perform the same optimization?
> I mean with the wi_fold_in_parts, if range of the divisor has 2 values and
> one of them is 0, won't it try to union range of X / 1 (range of X) and
> range of X / 0 (undefined) and yield range of X?  So say won't 7 /
> Y_with_bool_range yield
> [7,7] ?

It will... but ranger wont remove the divide unless the simplifier or folding
does it.

ie:

  if (x == 7 || x == 0)
    return y/x;

Produces:

x_5(D)  int [0, 0][7, 7]
    <bb 3> :
    _8 = y_7(D) / x_5(D);
    // predicted unlikely by early return (on trees) predictor.

_8 : int [-306783378, 306783378]

Change it to:

  if (y != 7)
    return 1;
  if (x == 7 || x == 0)
    return y/x;

and ranger will provide [7,7] / [0,0][7,7] then go thru folding and remove the
statement:

Folding statement: _8 = y_5(D) / x_6(D);
Queued stmt for removal.  Folds to: 1

however ranger will indeed calculate this range as [1,1]. _8 will be registered
as [1,1] and some follow on code may get eliminated as a result... 

Even if the folder didn't remove this, Its possible that if we propagate the
value of [1,1] it could make the divide dead code, and it could then be removed
by some other passes.

If we have states where divide by zero cannot possibly ever be eliminated like
this, then we could have the / 0 case return VARYING instead...   we just need
to decide in rangeops what behaviour you want.   In fact, I think it did in
GCC11...  yeah, here it is.

  // If we're definitely dividing by zero, there's nothing to do.
  if (wi_zero_p (type, divisor_min, divisor_max))
    {
      r.set_varying (type);
      return;
    }

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (39 preceding siblings ...)
  2022-02-03 18:35 ` amacleod at redhat dot com
@ 2022-02-04  7:08 ` rguenther at suse dot de
  2022-02-04  7:11 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenther at suse dot de @ 2022-02-04  7:08 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #40 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 3 Feb 2022, amacleod at redhat dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356
> 
> --- Comment #37 from Andrew Macleod <amacleod at redhat dot com> ---
> (In reply to Jakub Jelinek from comment #35)
> > I meant something like:
> >   return Z / X;
> 
> > and there evrp does with -O2 -gnatp optimize away the division.
> > Though that is likely the X / boolean_range_Y case which you've disabled.
> > In any case, I think you want to hear from Andrew/Aldy where exactly does
> > VRP/ranger assume UB on integer division by zero.
> 
> That divide is remove by the simplifier because it determines that X has a
> range of [0,1] and I believe the simplifer chooses to ignore the 0 under
> various circumstances.
> 
> As for ranger, range-ops will return UNDEFINED for the range if x is known to
> be [0,0].  This can be propagated around, and depending on how it ends up being
> used as to what happens with it. 

I think that's OK as outgoing range (on the non-exceptional path - on the
exeptional path the result isn't computed).  That just may not be used
to simplify the stmt producing the range itself of course.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (40 preceding siblings ...)
  2022-02-04  7:08 ` rguenther at suse dot de
@ 2022-02-04  7:11 ` rguenth at gcc dot gnu.org
  2022-02-04 10:05 ` ebotcazou at gcc dot gnu.org
                   ` (5 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-04  7:11 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #41 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #40)
> On Thu, 3 Feb 2022, amacleod at redhat dot com wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356
> > 
> > --- Comment #37 from Andrew Macleod <amacleod at redhat dot com> ---
> > (In reply to Jakub Jelinek from comment #35)
> > > I meant something like:
> > >   return Z / X;
> > 
> > > and there evrp does with -O2 -gnatp optimize away the division.
> > > Though that is likely the X / boolean_range_Y case which you've disabled.
> > > In any case, I think you want to hear from Andrew/Aldy where exactly does
> > > VRP/ranger assume UB on integer division by zero.
> > 
> > That divide is remove by the simplifier because it determines that X has a
> > range of [0,1] and I believe the simplifer chooses to ignore the 0 under
> > various circumstances.
> > 
> > As for ranger, range-ops will return UNDEFINED for the range if x is known to
> > be [0,0].  This can be propagated around, and depending on how it ends up being
> > used as to what happens with it. 
> 
> I think that's OK as outgoing range (on the non-exceptional path - on the
> exeptional path the result isn't computed).  That just may not be used
> to simplify the stmt producing the range itself of course.

That said, range-ops, from say

  [0,1] = [0,2] / y;

may _not_ reason that 'y' is not 0 when non-call EH.  That is, you need to be
careful on the reverse ops but I think not on the forward ops.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (41 preceding siblings ...)
  2022-02-04  7:11 ` rguenth at gcc dot gnu.org
@ 2022-02-04 10:05 ` ebotcazou at gcc dot gnu.org
  2022-02-04 11:10 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-04 10:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |ebotcazou at gcc dot gnu.org

--- Comment #42 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Fixing.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (42 preceding siblings ...)
  2022-02-04 10:05 ` ebotcazou at gcc dot gnu.org
@ 2022-02-04 11:10 ` cvs-commit at gcc dot gnu.org
  2022-02-04 11:17 ` ebotcazou at gcc dot gnu.org
                   ` (3 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-02-04 11:10 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #43 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Eric Botcazou <ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:bd14cdceb9c6f4800e25a9fbca635a1d4c06fd32

commit r12-7048-gbd14cdceb9c6f4800e25a9fbca635a1d4c06fd32
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Fri Feb 4 12:03:49 2022 +0100

    Disable new 1/X optimization with -fnon-call-exceptions

    The trapping behavior of the operation needs to be preserved when the
    -fnon-call-exceptions switch is in effect.  This also adds the same
    guards to similar optimizations.

    gcc/
            PR tree-optimization/104356
            * match.pd (X / bool_range_Y is X): Add guard.
            (X / X is one): Likewise.
            (X / abs (X) is X < 0 ? -1 : 1): Likewise.
            (X / -X is -1): Likewise.
            (1 / X -> X == 1): Likewise.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (43 preceding siblings ...)
  2022-02-04 11:10 ` cvs-commit at gcc dot gnu.org
@ 2022-02-04 11:17 ` ebotcazou at gcc dot gnu.org
  2022-02-04 13:46 ` amacleod at redhat dot com
                   ` (2 subsequent siblings)
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-04 11:17 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #44 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
I'll create a separate PR for the VRP issue.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (44 preceding siblings ...)
  2022-02-04 11:17 ` ebotcazou at gcc dot gnu.org
@ 2022-02-04 13:46 ` amacleod at redhat dot com
  2022-02-04 16:40 ` ebotcazou at gcc dot gnu.org
  2022-02-04 16:45 ` cvs-commit at gcc dot gnu.org
  47 siblings, 0 replies; 49+ messages in thread
From: amacleod at redhat dot com @ 2022-02-04 13:46 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #45 from Andrew Macleod <amacleod at redhat dot com> ---

> 
> That said, range-ops, from say
> 
>   [0,1] = [0,2] / y;
> 
> may _not_ reason that 'y' is not 0 when non-call EH.  That is, you need to be
> careful on the reverse ops but I think not on the forward ops.

We do not currently try to figure anything via reverse ops for general divide
operations.

For exact_divide we try if Y is a non-zero singleton, but that's about it.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (45 preceding siblings ...)
  2022-02-04 13:46 ` amacleod at redhat dot com
@ 2022-02-04 16:40 ` ebotcazou at gcc dot gnu.org
  2022-02-04 16:45 ` cvs-commit at gcc dot gnu.org
  47 siblings, 0 replies; 49+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2022-02-04 16:40 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #46 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I meant something like:
> with System.Unsigned_Types; use System.Unsigned_Types;
> 
> function F (X, Y : Unsigned) return Unsigned is
>   Z : Unsigned;
> begin
>   if X >=2 then
>     return 0;
>   end if;
>   Z := Y;
>   if X = 1 then
>     Z := Y + 4;
>   end if;
>   return Z / X;
> end;
> and there evrp does with -O2 -gnatp optimize away the division.

My bad.  I forgot that -gnatp now disables -fnon-call-exceptions too (in order
to make the mode mimic C++) so the testcase must be written:

with System.Unsigned_Types; use System.Unsigned_Types;

function Opt97 (X, Y : Unsigned) return Unsigned is

  pragma Suppress (All_Checks);

  Z : Unsigned;

begin
  if X >= 2 then
    return 0;
  end if;

  Z := Y;
  if X = 1 then
    Z := Y + 4;
  end if;

  return Z / X;
end;

and there is still the divide at -O2 and above on mainline.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away
  2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
                   ` (46 preceding siblings ...)
  2022-02-04 16:40 ` ebotcazou at gcc dot gnu.org
@ 2022-02-04 16:45 ` cvs-commit at gcc dot gnu.org
  47 siblings, 0 replies; 49+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-02-04 16:45 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104356

--- Comment #47 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Eric Botcazou <ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:1f722e35ab3805de6eeace770508a9085944e93e

commit r12-7058-g1f722e35ab3805de6eeace770508a9085944e93e
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Fri Feb 4 17:41:55 2022 +0100

    Add optmization testcase for incorrect optimization in Ada

    gcc/testsuite/
            PR tree-optimization/104356
            * gnat.dg/opt97.adb: New test.

^ permalink raw reply	[flat|nested] 49+ messages in thread

end of thread, other threads:[~2022-02-04 16:45 UTC | newest]

Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-02 22:56 [Bug tree-optimization/104356] New: [12 Regression] divide by zero trap is being removed now when it should not be in some cases pinskia at gcc dot gnu.org
2022-02-02 22:57 ` [Bug tree-optimization/104356] " pinskia at gcc dot gnu.org
2022-02-02 23:08 ` jakub at gcc dot gnu.org
2022-02-03  7:32 ` rguenth at gcc dot gnu.org
2022-02-03  7:42 ` pinskia at gcc dot gnu.org
2022-02-03  7:43 ` pinskia at gcc dot gnu.org
2022-02-03  7:58 ` pinskia at gcc dot gnu.org
2022-02-03  8:34 ` rguenther at suse dot de
2022-02-03 10:40 ` ebotcazou at gcc dot gnu.org
2022-02-03 11:08 ` rguenther at suse dot de
2022-02-03 11:14 ` ebotcazou at gcc dot gnu.org
2022-02-03 11:22 ` jakub at gcc dot gnu.org
2022-02-03 11:41 ` ebotcazou at gcc dot gnu.org
2022-02-03 11:43 ` rguenth at gcc dot gnu.org
2022-02-03 11:45 ` jakub at gcc dot gnu.org
2022-02-03 11:48 ` ebotcazou at gcc dot gnu.org
2022-02-03 11:50 ` ebotcazou at gcc dot gnu.org
2022-02-03 11:52 ` rguenth at gcc dot gnu.org
2022-02-03 11:56 ` rguenth at gcc dot gnu.org
2022-02-03 12:00 ` rguenth at gcc dot gnu.org
2022-02-03 12:00 ` jakub at gcc dot gnu.org
2022-02-03 12:03 ` jakub at gcc dot gnu.org
2022-02-03 12:03 ` ebotcazou at gcc dot gnu.org
2022-02-03 12:05 ` ebotcazou at gcc dot gnu.org
2022-02-03 12:05 ` rguenther at suse dot de
2022-02-03 12:10 ` rguenth at gcc dot gnu.org
2022-02-03 12:13 ` jakub at gcc dot gnu.org
2022-02-03 12:18 ` ebotcazou at gcc dot gnu.org
2022-02-03 12:21 ` cvs-commit at gcc dot gnu.org
2022-02-03 12:28 ` ebotcazou at gcc dot gnu.org
2022-02-03 12:39 ` jakub at gcc dot gnu.org
2022-02-03 12:49 ` ebotcazou at gcc dot gnu.org
2022-02-03 12:51 ` ebotcazou at gcc dot gnu.org
2022-02-03 13:04 ` jakub at gcc dot gnu.org
2022-02-03 13:09 ` rguenther at suse dot de
2022-02-03 15:41 ` ebotcazou at gcc dot gnu.org
2022-02-03 16:28 ` [Bug tree-optimization/104356] [12 Regression] divide by zero trap incorrectly optimized away jakub at gcc dot gnu.org
2022-02-03 16:41 ` ebotcazou at gcc dot gnu.org
2022-02-03 16:51 ` amacleod at redhat dot com
2022-02-03 16:57 ` jakub at gcc dot gnu.org
2022-02-03 18:35 ` amacleod at redhat dot com
2022-02-04  7:08 ` rguenther at suse dot de
2022-02-04  7:11 ` rguenth at gcc dot gnu.org
2022-02-04 10:05 ` ebotcazou at gcc dot gnu.org
2022-02-04 11:10 ` cvs-commit at gcc dot gnu.org
2022-02-04 11:17 ` ebotcazou at gcc dot gnu.org
2022-02-04 13:46 ` amacleod at redhat dot com
2022-02-04 16:40 ` ebotcazou at gcc dot gnu.org
2022-02-04 16:45 ` cvs-commit at gcc dot gnu.org

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).