[PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[Aldy Hernandez]

As discussed in the PR.  The code makes no difference, so whatever test
we added this special case for has been fixed or is being papered over.
I think we should fix any fall out upstream.

[Unless Andrew can remember why we added this and it still applies.]

Tested on x86-64 Linux.

OK for trunk?

	PR 103451

gcc/ChangeLog:

	* range-op.cc (operator_div::wi_fold): Remove
	can_throw_non_call_exceptions special case.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr103451.c: New test.
---
 gcc/range-op.cc                 |  7 -------
 gcc/testsuite/gcc.dg/pr103451.c | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr103451.c

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index bbf2924f815..6fe5f1cb4e0 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -1832,13 +1832,6 @@ operator_div::wi_fold (irange &r, tree type,
       return;
     }
 
-  // If flag_non_call_exceptions, we must not eliminate a division by zero.
-  if (cfun->can_throw_non_call_exceptions)
-    {
-      r.set_varying (type);
-      return;
-    }
-
   // If we're definitely dividing by zero, there's nothing to do.
   if (wi_zero_p (type, divisor_min, divisor_max))
     {
diff --git a/gcc/testsuite/gcc.dg/pr103451.c b/gcc/testsuite/gcc.dg/pr103451.c
new file mode 100644
index 00000000000..b83646d0b83
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr103451.c
@@ -0,0 +1,17 @@
+// { dg-do compile }
+// { dg-options "-O2 -w" }
+
+int func_10_ptr_12;
+
+void func_10(long li_8) 
+{
+  long *ptr_9 = &li_8;
+  li_8 &= *ptr_9 / 0 ?: li_8;
+  for (;;)
+    func_10_ptr_12 &= 4 ? *ptr_9 : 4;
+}
+
+void func_9_s_8() 
+{ 
+  func_10(func_9_s_8); 
+}
-- 
2.31.1

Re: [PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[Jeff Law]

On 11/29/2021 7:00 AM, Aldy Hernandez via Gcc-patches wrote:
> As discussed in the PR.  The code makes no difference, so whatever test
> we added this special case for has been fixed or is being papered over.
> I think we should fix any fall out upstream.
>
> [Unless Andrew can remember why we added this and it still applies.]
>
> Tested on x86-64 Linux.
>
> OK for trunk?
>
> 	PR 103451
>
> gcc/ChangeLog:
>
> 	* range-op.cc (operator_div::wi_fold): Remove
> 	can_throw_non_call_exceptions special case.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/pr103451.c: New test.
I'll defer to Andrew, but it seems wrong to me.  The whole point is to 
set the result to varying so that we don't know the result and never 
remove the division which is critical for -fnon-call-exceptions.


> ---
>   gcc/range-op.cc                 |  7 -------
>   gcc/testsuite/gcc.dg/pr103451.c | 17 +++++++++++++++++
>   2 files changed, 17 insertions(+), 7 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/pr103451.c
>
> diff --git a/gcc/range-op.cc b/gcc/range-op.cc
> index bbf2924f815..6fe5f1cb4e0 100644
> --- a/gcc/range-op.cc
> +++ b/gcc/range-op.cc
> @@ -1832,13 +1832,6 @@ operator_div::wi_fold (irange &r, tree type,
>         return;
>       }
>   
> -  // If flag_non_call_exceptions, we must not eliminate a division by zero.
> -  if (cfun->can_throw_non_call_exceptions)
> -    {
> -      r.set_varying (type);
> -      return;
> -    }
> -
>     // If we're definitely dividing by zero, there's nothing to do.
>     if (wi_zero_p (type, divisor_min, divisor_max))
>       {
> diff --git a/gcc/testsuite/gcc.dg/pr103451.c b/gcc/testsuite/gcc.dg/pr103451.c
> new file mode 100644
> index 00000000000..b83646d0b83
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr103451.c
> @@ -0,0 +1,17 @@
> +// { dg-do compile }
> +// { dg-options "-O2 -w" }
ISTM that what you want to test for is that the division by zero remains 
in the IL for -fnon-call-exceptions.

jeff

Re: [PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[Richard Biener]

On Mon, Nov 29, 2021 at 3:39 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 11/29/2021 7:00 AM, Aldy Hernandez via Gcc-patches wrote:
> > As discussed in the PR.  The code makes no difference, so whatever test
> > we added this special case for has been fixed or is being papered over.
> > I think we should fix any fall out upstream.
> >
> > [Unless Andrew can remember why we added this and it still applies.]
> >
> > Tested on x86-64 Linux.
> >
> > OK for trunk?
> >
> >       PR 103451
> >
> > gcc/ChangeLog:
> >
> >       * range-op.cc (operator_div::wi_fold): Remove
> >       can_throw_non_call_exceptions special case.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/pr103451.c: New test.
> I'll defer to Andrew, but it seems wrong to me.  The whole point is to
> set the result to varying so that we don't know the result and never
> remove the division which is critical for -fnon-call-exceptions.

But that has nothing to do with computing the value range for
the result which is only accessible when the stmt does _not_ throw ...

That is, if we compute non-VARYING here and because of that
remove the stmt then _that's_ the place to fix (IMO)

>
> > ---
> >   gcc/range-op.cc                 |  7 -------
> >   gcc/testsuite/gcc.dg/pr103451.c | 17 +++++++++++++++++
> >   2 files changed, 17 insertions(+), 7 deletions(-)
> >   create mode 100644 gcc/testsuite/gcc.dg/pr103451.c
> >
> > diff --git a/gcc/range-op.cc b/gcc/range-op.cc
> > index bbf2924f815..6fe5f1cb4e0 100644
> > --- a/gcc/range-op.cc
> > +++ b/gcc/range-op.cc
> > @@ -1832,13 +1832,6 @@ operator_div::wi_fold (irange &r, tree type,
> >         return;
> >       }
> >
> > -  // If flag_non_call_exceptions, we must not eliminate a division by zero.
> > -  if (cfun->can_throw_non_call_exceptions)
> > -    {
> > -      r.set_varying (type);
> > -      return;
> > -    }
> > -
> >     // If we're definitely dividing by zero, there's nothing to do.
> >     if (wi_zero_p (type, divisor_min, divisor_max))
> >       {
> > diff --git a/gcc/testsuite/gcc.dg/pr103451.c b/gcc/testsuite/gcc.dg/pr103451.c
> > new file mode 100644
> > index 00000000000..b83646d0b83
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr103451.c
> > @@ -0,0 +1,17 @@
> > +// { dg-do compile }
> > +// { dg-options "-O2 -w" }
> ISTM that what you want to test for is that the division by zero remains
> in the IL for -fnon-call-exceptions.
>
> jeff
>

Re: [PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1703 bytes --]

On Mon, Nov 29, 2021 at 3:48 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Mon, Nov 29, 2021 at 3:39 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> >
> >
> >
> > On 11/29/2021 7:00 AM, Aldy Hernandez via Gcc-patches wrote:
> > > As discussed in the PR.  The code makes no difference, so whatever test
> > > we added this special case for has been fixed or is being papered over.
> > > I think we should fix any fall out upstream.
> > >
> > > [Unless Andrew can remember why we added this and it still applies.]
> > >
> > > Tested on x86-64 Linux.
> > >
> > > OK for trunk?
> > >
> > >       PR 103451
> > >
> > > gcc/ChangeLog:
> > >
> > >       * range-op.cc (operator_div::wi_fold): Remove
> > >       can_throw_non_call_exceptions special case.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >       * gcc.dg/pr103451.c: New test.
> > I'll defer to Andrew, but it seems wrong to me.  The whole point is to
> > set the result to varying so that we don't know the result and never
> > remove the division which is critical for -fnon-call-exceptions.
>
> But that has nothing to do with computing the value range for
> the result which is only accessible when the stmt does _not_ throw ...
>
> That is, if we compute non-VARYING here and because of that
> remove the stmt then _that's_ the place to fix (IMO)

Ughh, I think you're both right.

We should fix this upstream AND we should test for the presence of the
division by 0 in the optimized dump.

Of course doing both opens a can of worms.  The division by zero can
be cleaned up by (at least) DCE, DSE, and the code sinking passes.
I've fixed all 3 in the attached (untested) patch.  Dunno what y'all
want to do at this point.

Aldy

[-- Attachment #2: 0001-Remove-can_throw_non_call_exceptions-special-case-fr.patch --]
[-- Type: text/x-patch, Size: 3762 bytes --]

From c521bd22d4a7360c7b01f864392eb5cf68cfc6f0 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Mon, 29 Nov 2021 12:52:45 +0100
Subject: [PATCH] Remove can_throw_non_call_exceptions special case from
 operator_div::wi_fold.

	PR 103451

gcc/ChangeLog:

	* range-op.cc (operator_div::wi_fold): Remove
	can_throw_non_call_exceptions special case.
	* tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Check for
	can_throw_non_call_exceptions.
	* tree-ssa-dse.c (pass_dse::execute): Same.
	* tree-ssa-sink.c (sink_code_in_bb): Same.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr103451.c: New test.
---
 gcc/range-op.cc                 |  7 -------
 gcc/testsuite/gcc.dg/pr103451.c | 19 +++++++++++++++++++
 gcc/tree-ssa-dce.c              |  3 ++-
 gcc/tree-ssa-dse.c              |  3 ++-
 gcc/tree-ssa-sink.c             |  4 +++-
 5 files changed, 26 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr103451.c

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index bbf2924f815..6fe5f1cb4e0 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -1832,13 +1832,6 @@ operator_div::wi_fold (irange &r, tree type,
       return;
     }
 
-  // If flag_non_call_exceptions, we must not eliminate a division by zero.
-  if (cfun->can_throw_non_call_exceptions)
-    {
-      r.set_varying (type);
-      return;
-    }
-
   // If we're definitely dividing by zero, there's nothing to do.
   if (wi_zero_p (type, divisor_min, divisor_max))
     {
diff --git a/gcc/testsuite/gcc.dg/pr103451.c b/gcc/testsuite/gcc.dg/pr103451.c
new file mode 100644
index 00000000000..90a584490ae
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr103451.c
@@ -0,0 +1,19 @@
+// { dg-do compile }
+// { dg-options "-O2 -w -fnon-call-exceptions -fdump-tree-optimized" }
+
+int func_10_ptr_12;
+
+void func_10(long li_8) 
+{
+  long *ptr_9 = &li_8;
+  li_8 &= *ptr_9 / 0 ?: li_8;
+  for (;;)
+    func_10_ptr_12 &= 4 ? *ptr_9 : 4;
+}
+
+void func_9_s_8() 
+{ 
+  func_10(func_9_s_8); 
+}
+
+// { dg-final { scan-tree-dump " / 0" "optimized" } }
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index 1f817b95fab..1c1a5cc0811 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -304,7 +304,8 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
   /* If a statement could throw, it can be deemed necessary unless we
      are allowed to remove dead EH.  Test this after checking for
      new/delete operators since we always elide their EH.  */
-  if (!cfun->can_delete_dead_exceptions
+  if ((!cfun->can_delete_dead_exceptions
+       || cfun->can_throw_non_call_exceptions)
       && stmt_could_throw_p (cfun, stmt))
     {
       mark_stmt_necessary (stmt, true);
diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 8717d654e5a..aa16565b429 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -1446,7 +1446,8 @@ pass_dse::execute (function *fun)
 		  && !gimple_has_side_effects (stmt)
 		  && !is_ctrl_altering_stmt (stmt)
 		  && (!stmt_could_throw_p (fun, stmt)
-		      || fun->can_delete_dead_exceptions))
+		      || (fun->can_delete_dead_exceptions
+			  && !cfun->can_throw_non_call_exceptions)))
 		{
 		  if (dump_file && (dump_flags & TDF_DETAILS))
 		    {
diff --git a/gcc/tree-ssa-sink.c b/gcc/tree-ssa-sink.c
index 92f444ec1c8..29299129cfa 100644
--- a/gcc/tree-ssa-sink.c
+++ b/gcc/tree-ssa-sink.c
@@ -696,7 +696,9 @@ sink_code_in_bb (basic_block bb)
 	  /* If we face a dead stmt remove it as it possibly blocks
 	     sinking of uses.  */
 	  if (zero_uses_p
-	      && ! gimple_vdef (stmt))
+	      && !gimple_vdef (stmt)
+	      && (cfun->can_delete_dead_exceptions
+		  && !cfun->can_throw_non_call_exceptions))
 	    {
 	      gsi_remove (&saved, true);
 	      release_defs (stmt);
-- 
2.31.1

Re: [PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[Richard Biener]

On Mon, Nov 29, 2021 at 4:24 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> On Mon, Nov 29, 2021 at 3:48 PM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Mon, Nov 29, 2021 at 3:39 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> > >
> > >
> > >
> > > On 11/29/2021 7:00 AM, Aldy Hernandez via Gcc-patches wrote:
> > > > As discussed in the PR.  The code makes no difference, so whatever test
> > > > we added this special case for has been fixed or is being papered over.
> > > > I think we should fix any fall out upstream.
> > > >
> > > > [Unless Andrew can remember why we added this and it still applies.]
> > > >
> > > > Tested on x86-64 Linux.
> > > >
> > > > OK for trunk?
> > > >
> > > >       PR 103451
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > >       * range-op.cc (operator_div::wi_fold): Remove
> > > >       can_throw_non_call_exceptions special case.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > >       * gcc.dg/pr103451.c: New test.
> > > I'll defer to Andrew, but it seems wrong to me.  The whole point is to
> > > set the result to varying so that we don't know the result and never
> > > remove the division which is critical for -fnon-call-exceptions.
> >
> > But that has nothing to do with computing the value range for
> > the result which is only accessible when the stmt does _not_ throw ...
> >
> > That is, if we compute non-VARYING here and because of that
> > remove the stmt then _that's_ the place to fix (IMO)
>
> Ughh, I think you're both right.
>
> We should fix this upstream AND we should test for the presence of the
> division by 0 in the optimized dump.
>
> Of course doing both opens a can of worms.  The division by zero can
> be cleaned up by (at least) DCE, DSE, and the code sinking passes.
> I've fixed all 3 in the attached (untested) patch.  Dunno what y'all
> want to do at this point.

I think you need to add -fno-delete-dead-exceptions to the testcase.
The sinking
bug looks real, but just

         && (cfun->can_delete_dead_exceptions
                || !stmt_could_throw_p (cfun, stmt))

is needed there.  That change is OK.

Thanks,
Richard.

>
> Aldy

Re: [PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 2395 bytes --]

On Tue, Nov 30, 2021 at 8:37 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Mon, Nov 29, 2021 at 4:24 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > On Mon, Nov 29, 2021 at 3:48 PM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > >
> > > On Mon, Nov 29, 2021 at 3:39 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> > > >
> > > >
> > > >
> > > > On 11/29/2021 7:00 AM, Aldy Hernandez via Gcc-patches wrote:
> > > > > As discussed in the PR.  The code makes no difference, so whatever test
> > > > > we added this special case for has been fixed or is being papered over.
> > > > > I think we should fix any fall out upstream.
> > > > >
> > > > > [Unless Andrew can remember why we added this and it still applies.]
> > > > >
> > > > > Tested on x86-64 Linux.
> > > > >
> > > > > OK for trunk?
> > > > >
> > > > >       PR 103451
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > >       * range-op.cc (operator_div::wi_fold): Remove
> > > > >       can_throw_non_call_exceptions special case.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > >       * gcc.dg/pr103451.c: New test.
> > > > I'll defer to Andrew, but it seems wrong to me.  The whole point is to
> > > > set the result to varying so that we don't know the result and never
> > > > remove the division which is critical for -fnon-call-exceptions.
> > >
> > > But that has nothing to do with computing the value range for
> > > the result which is only accessible when the stmt does _not_ throw ...
> > >
> > > That is, if we compute non-VARYING here and because of that
> > > remove the stmt then _that's_ the place to fix (IMO)
> >
> > Ughh, I think you're both right.
> >
> > We should fix this upstream AND we should test for the presence of the
> > division by 0 in the optimized dump.
> >
> > Of course doing both opens a can of worms.  The division by zero can
> > be cleaned up by (at least) DCE, DSE, and the code sinking passes.
> > I've fixed all 3 in the attached (untested) patch.  Dunno what y'all
> > want to do at this point.
>
> I think you need to add -fno-delete-dead-exceptions to the testcase.
> The sinking
> bug looks real, but just
>
>          && (cfun->can_delete_dead_exceptions
>                 || !stmt_could_throw_p (cfun, stmt))
>
> is needed there.  That change is OK.

Did you mean the entire patch (as attached) is OK, or just the sink part?

Thanks.
Aldy

[-- Attachment #2: 0001-Remove-can_throw_non_call_exceptions-special-case-fr.patch --]
[-- Type: text/x-patch, Size: 3786 bytes --]

From e0abd7b05709e41b2e2fda5bde7f5802c6d953ef Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Mon, 29 Nov 2021 12:52:45 +0100
Subject: [PATCH] Remove can_throw_non_call_exceptions special case from
 operator_div::wi_fold.

	PR 103451

gcc/ChangeLog:

	* range-op.cc (operator_div::wi_fold): Remove
	can_throw_non_call_exceptions special case.
	* tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Check for
	can_throw_non_call_exceptions.
	* tree-ssa-dse.c (pass_dse::execute): Same.
	* tree-ssa-sink.c (sink_code_in_bb): Same.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr103451.c: New test.
---
 gcc/range-op.cc                 |  7 -------
 gcc/testsuite/gcc.dg/pr103451.c | 19 +++++++++++++++++++
 gcc/tree-ssa-dce.c              |  3 ++-
 gcc/tree-ssa-dse.c              |  3 ++-
 gcc/tree-ssa-sink.c             |  4 +++-
 5 files changed, 26 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr103451.c

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index bbf2924f815..6fe5f1cb4e0 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -1832,13 +1832,6 @@ operator_div::wi_fold (irange &r, tree type,
       return;
     }
 
-  // If flag_non_call_exceptions, we must not eliminate a division by zero.
-  if (cfun->can_throw_non_call_exceptions)
-    {
-      r.set_varying (type);
-      return;
-    }
-
   // If we're definitely dividing by zero, there's nothing to do.
   if (wi_zero_p (type, divisor_min, divisor_max))
     {
diff --git a/gcc/testsuite/gcc.dg/pr103451.c b/gcc/testsuite/gcc.dg/pr103451.c
new file mode 100644
index 00000000000..c701934603e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr103451.c
@@ -0,0 +1,19 @@
+// { dg-do compile }
+// { dg-options "-O2 -w -fnon-call-exceptions -fno-delete-dead-exceptions -fdump-tree-optimized" }
+
+int func_10_ptr_12;
+
+void func_10(long li_8) 
+{
+  long *ptr_9 = &li_8;
+  li_8 &= *ptr_9 / 0 ?: li_8;
+  for (;;)
+    func_10_ptr_12 &= 4 ? *ptr_9 : 4;
+}
+
+void func_9_s_8() 
+{ 
+  func_10(func_9_s_8); 
+}
+
+// { dg-final { scan-tree-dump " / 0" "optimized" } }
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index 1f817b95fab..1c1a5cc0811 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -304,7 +304,8 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
   /* If a statement could throw, it can be deemed necessary unless we
      are allowed to remove dead EH.  Test this after checking for
      new/delete operators since we always elide their EH.  */
-  if (!cfun->can_delete_dead_exceptions
+  if ((!cfun->can_delete_dead_exceptions
+       || cfun->can_throw_non_call_exceptions)
       && stmt_could_throw_p (cfun, stmt))
     {
       mark_stmt_necessary (stmt, true);
diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 8717d654e5a..aa16565b429 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -1446,7 +1446,8 @@ pass_dse::execute (function *fun)
 		  && !gimple_has_side_effects (stmt)
 		  && !is_ctrl_altering_stmt (stmt)
 		  && (!stmt_could_throw_p (fun, stmt)
-		      || fun->can_delete_dead_exceptions))
+		      || (fun->can_delete_dead_exceptions
+			  && !cfun->can_throw_non_call_exceptions)))
 		{
 		  if (dump_file && (dump_flags & TDF_DETAILS))
 		    {
diff --git a/gcc/tree-ssa-sink.c b/gcc/tree-ssa-sink.c
index 92f444ec1c8..c5d67840be3 100644
--- a/gcc/tree-ssa-sink.c
+++ b/gcc/tree-ssa-sink.c
@@ -696,7 +696,9 @@ sink_code_in_bb (basic_block bb)
 	  /* If we face a dead stmt remove it as it possibly blocks
 	     sinking of uses.  */
 	  if (zero_uses_p
-	      && ! gimple_vdef (stmt))
+	      && !gimple_vdef (stmt)
+	      && (cfun->can_delete_dead_exceptions
+		  || !stmt_could_throw_p (cfun, stmt)))
 	    {
 	      gsi_remove (&saved, true);
 	      release_defs (stmt);
-- 
2.31.1

Re: [PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[Richard Biener]

On Tue, Nov 30, 2021 at 9:51 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> On Tue, Nov 30, 2021 at 8:37 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Mon, Nov 29, 2021 at 4:24 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> > >
> > > On Mon, Nov 29, 2021 at 3:48 PM Richard Biener
> > > <richard.guenther@gmail.com> wrote:
> > > >
> > > > On Mon, Nov 29, 2021 at 3:39 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 11/29/2021 7:00 AM, Aldy Hernandez via Gcc-patches wrote:
> > > > > > As discussed in the PR.  The code makes no difference, so whatever test
> > > > > > we added this special case for has been fixed or is being papered over.
> > > > > > I think we should fix any fall out upstream.
> > > > > >
> > > > > > [Unless Andrew can remember why we added this and it still applies.]
> > > > > >
> > > > > > Tested on x86-64 Linux.
> > > > > >
> > > > > > OK for trunk?
> > > > > >
> > > > > >       PR 103451
> > > > > >
> > > > > > gcc/ChangeLog:
> > > > > >
> > > > > >       * range-op.cc (operator_div::wi_fold): Remove
> > > > > >       can_throw_non_call_exceptions special case.
> > > > > >
> > > > > > gcc/testsuite/ChangeLog:
> > > > > >
> > > > > >       * gcc.dg/pr103451.c: New test.
> > > > > I'll defer to Andrew, but it seems wrong to me.  The whole point is to
> > > > > set the result to varying so that we don't know the result and never
> > > > > remove the division which is critical for -fnon-call-exceptions.
> > > >
> > > > But that has nothing to do with computing the value range for
> > > > the result which is only accessible when the stmt does _not_ throw ...
> > > >
> > > > That is, if we compute non-VARYING here and because of that
> > > > remove the stmt then _that's_ the place to fix (IMO)
> > >
> > > Ughh, I think you're both right.
> > >
> > > We should fix this upstream AND we should test for the presence of the
> > > division by 0 in the optimized dump.
> > >
> > > Of course doing both opens a can of worms.  The division by zero can
> > > be cleaned up by (at least) DCE, DSE, and the code sinking passes.
> > > I've fixed all 3 in the attached (untested) patch.  Dunno what y'all
> > > want to do at this point.
> >
> > I think you need to add -fno-delete-dead-exceptions to the testcase.
> > The sinking
> > bug looks real, but just
> >
> >          && (cfun->can_delete_dead_exceptions
> >                 || !stmt_could_throw_p (cfun, stmt))
> >
> > is needed there.  That change is OK.
>
> Did you mean the entire patch (as attached) is OK, or just the sink part?

The DCE and DSE parts are wrong and not needed.  The remaining pieces
are OK.

Thanks,
Richard.

> Thanks.
> Aldy

Re: [PATCH] Remove can_throw_non_call_exceptions special case from operator_div::wi_fold.

[Aldy Hernandez]

Will adjust, re-test and commit.

Thanks.
Aldy

On Tue, Nov 30, 2021 at 10:00 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Nov 30, 2021 at 9:51 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > On Tue, Nov 30, 2021 at 8:37 AM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > >
> > > On Mon, Nov 29, 2021 at 4:24 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> > > >
> > > > On Mon, Nov 29, 2021 at 3:48 PM Richard Biener
> > > > <richard.guenther@gmail.com> wrote:
> > > > >
> > > > > On Mon, Nov 29, 2021 at 3:39 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 11/29/2021 7:00 AM, Aldy Hernandez via Gcc-patches wrote:
> > > > > > > As discussed in the PR.  The code makes no difference, so whatever test
> > > > > > > we added this special case for has been fixed or is being papered over.
> > > > > > > I think we should fix any fall out upstream.
> > > > > > >
> > > > > > > [Unless Andrew can remember why we added this and it still applies.]
> > > > > > >
> > > > > > > Tested on x86-64 Linux.
> > > > > > >
> > > > > > > OK for trunk?
> > > > > > >
> > > > > > >       PR 103451
> > > > > > >
> > > > > > > gcc/ChangeLog:
> > > > > > >
> > > > > > >       * range-op.cc (operator_div::wi_fold): Remove
> > > > > > >       can_throw_non_call_exceptions special case.
> > > > > > >
> > > > > > > gcc/testsuite/ChangeLog:
> > > > > > >
> > > > > > >       * gcc.dg/pr103451.c: New test.
> > > > > > I'll defer to Andrew, but it seems wrong to me.  The whole point is to
> > > > > > set the result to varying so that we don't know the result and never
> > > > > > remove the division which is critical for -fnon-call-exceptions.
> > > > >
> > > > > But that has nothing to do with computing the value range for
> > > > > the result which is only accessible when the stmt does _not_ throw ...
> > > > >
> > > > > That is, if we compute non-VARYING here and because of that
> > > > > remove the stmt then _that's_ the place to fix (IMO)
> > > >
> > > > Ughh, I think you're both right.
> > > >
> > > > We should fix this upstream AND we should test for the presence of the
> > > > division by 0 in the optimized dump.
> > > >
> > > > Of course doing both opens a can of worms.  The division by zero can
> > > > be cleaned up by (at least) DCE, DSE, and the code sinking passes.
> > > > I've fixed all 3 in the attached (untested) patch.  Dunno what y'all
> > > > want to do at this point.
> > >
> > > I think you need to add -fno-delete-dead-exceptions to the testcase.
> > > The sinking
> > > bug looks real, but just
> > >
> > >          && (cfun->can_delete_dead_exceptions
> > >                 || !stmt_could_throw_p (cfun, stmt))
> > >
> > > is needed there.  That change is OK.
> >
> > Did you mean the entire patch (as attached) is OK, or just the sink part?
>
> The DCE and DSE parts are wrong and not needed.  The remaining pieces
> are OK.
>
> Thanks,
> Richard.
>
> > Thanks.
> > Aldy
>