[1/4] ipa-sra: Do not remove statements necessary because of non-call EH (PR 95113)

Message ID 4450ddf43c03fafd4e527094a72c3c0d106f35c3.1590667594.git.mjambor@suse.cz
State New
Headers show
Series
  • Make IPA-SRA not depend on tree-dce and related fixes
Related show

Commit Message

Martin Jambor May 28, 2020, 12:06 p.m.
PR 95113 revealed that when reasoning about which parameters are dead,
IPA-SRA does not perform the same check related to non-call exceptions
as tree DCE.  It most certainly should and so this patch moves the
condition used in tree-ssa-dce.c into a separate predicate (in
tree-eh.c) and uses it from both places.

gcc/ChangeLog:

2020-05-27  Martin Jambor  <mjambor@suse.cz>

	PR ipa/95113
	* gcc/tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Move non-call
	exceptions check to...
	* gcc/tree-eh.c (stmt_unremovable_because_of_non_call_eh_p): ...this
	new function.
	* gcc/tree-eh.h (stmt_unremovable_because_of_non_call_eh_p): Declare it.
	* gcc/ipa-sra.c (isra_track_scalar_value_uses): Use it.  New parameter
	fun.

gcc/testsuite/ChangeLog:

2020-05-27  Martin Jambor  <mjambor@suse.cz>

	PR ipa/95113
	* gcc.dg/ipa/pr95113.c: New test.
---
 gcc/ipa-sra.c                      | 28 +++++++++++++------------
 gcc/testsuite/gcc.dg/ipa/pr95113.c | 33 ++++++++++++++++++++++++++++++
 gcc/tree-eh.c                      | 10 +++++++++
 gcc/tree-eh.h                      |  1 +
 gcc/tree-ssa-dce.c                 |  4 +---
 5 files changed, 60 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr95113.c

-- 
2.26.2

Comments

Richard Biener June 2, 2020, 11:42 a.m. | #1
On Thu, 28 May 2020, Martin Jambor wrote:

> PR 95113 revealed that when reasoning about which parameters are dead,

> IPA-SRA does not perform the same check related to non-call exceptions

> as tree DCE.  It most certainly should and so this patch moves the

> condition used in tree-ssa-dce.c into a separate predicate (in

> tree-eh.c) and uses it from both places.


OK.

Thanks,
Richard.

> gcc/ChangeLog:

> 

> 2020-05-27  Martin Jambor  <mjambor@suse.cz>

> 

> 	PR ipa/95113

> 	* gcc/tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Move non-call

> 	exceptions check to...

> 	* gcc/tree-eh.c (stmt_unremovable_because_of_non_call_eh_p): ...this

> 	new function.

> 	* gcc/tree-eh.h (stmt_unremovable_because_of_non_call_eh_p): Declare it.

> 	* gcc/ipa-sra.c (isra_track_scalar_value_uses): Use it.  New parameter

> 	fun.

> 

> gcc/testsuite/ChangeLog:

> 

> 2020-05-27  Martin Jambor  <mjambor@suse.cz>

> 

> 	PR ipa/95113

> 	* gcc.dg/ipa/pr95113.c: New test.

> ---

>  gcc/ipa-sra.c                      | 28 +++++++++++++------------

>  gcc/testsuite/gcc.dg/ipa/pr95113.c | 33 ++++++++++++++++++++++++++++++

>  gcc/tree-eh.c                      | 10 +++++++++

>  gcc/tree-eh.h                      |  1 +

>  gcc/tree-ssa-dce.c                 |  4 +---

>  5 files changed, 60 insertions(+), 16 deletions(-)

>  create mode 100644 gcc/testsuite/gcc.dg/ipa/pr95113.c

> 

> diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c

> index 7c922e40a4e..c81e8869e7a 100644

> --- a/gcc/ipa-sra.c

> +++ b/gcc/ipa-sra.c

> @@ -795,17 +795,17 @@ get_single_param_flow_source (const isra_param_flow *param_flow)

>  }

>  

>  /* Inspect all uses of NAME and simple arithmetic calculations involving NAME

> -   in NODE and return a negative number if any of them is used for something

> -   else than either an actual call argument, simple arithmetic operation or

> -   debug statement.  If there are no such uses, return the number of actual

> -   arguments that this parameter eventually feeds to (or zero if there is none).

> -   For any such parameter, mark PARM_NUM as one of its sources.  ANALYZED is a

> -   bitmap that tracks which SSA names we have already started

> -   investigating.  */

> +   in FUN represented with NODE and return a negative number if any of them is

> +   used for something else than either an actual call argument, simple

> +   arithmetic operation or debug statement.  If there are no such uses, return

> +   the number of actual arguments that this parameter eventually feeds to (or

> +   zero if there is none).  For any such parameter, mark PARM_NUM as one of its

> +   sources.  ANALYZED is a bitmap that tracks which SSA names we have already

> +   started investigating.  */

>  

>  static int

> -isra_track_scalar_value_uses (cgraph_node *node, tree name, int parm_num,

> -			      bitmap analyzed)

> +isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name,

> +			      int parm_num, bitmap analyzed)

>  {

>    int res = 0;

>    imm_use_iterator imm_iter;

> @@ -859,8 +859,9 @@ isra_track_scalar_value_uses (cgraph_node *node, tree name, int parm_num,

>  	    }

>  	  res += all_uses;

>  	}

> -      else if ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt))

> -	       || gimple_code (stmt) == GIMPLE_PHI)

> +      else if (!stmt_unremovable_because_of_non_call_eh_p (fun, stmt)

> +	       && ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt))

> +		   || gimple_code (stmt) == GIMPLE_PHI))

>  	{

>  	  tree lhs;

>  	  if (gimple_code (stmt) == GIMPLE_PHI)

> @@ -876,7 +877,7 @@ isra_track_scalar_value_uses (cgraph_node *node, tree name, int parm_num,

>  	  gcc_assert (!gimple_vdef (stmt));

>  	  if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs)))

>  	    {

> -	      int tmp = isra_track_scalar_value_uses (node, lhs, parm_num,

> +	      int tmp = isra_track_scalar_value_uses (fun, node, lhs, parm_num,

>  						      analyzed);

>  	      if (tmp < 0)

>  		{

> @@ -927,7 +928,8 @@ isra_track_scalar_param_local_uses (function *fun, cgraph_node *node, tree parm,

>      return true;

>  

>    bitmap analyzed = BITMAP_ALLOC (NULL);

> -  int call_uses = isra_track_scalar_value_uses (node, name, parm_num, analyzed);

> +  int call_uses = isra_track_scalar_value_uses (fun, node, name, parm_num,

> +						analyzed);

>    BITMAP_FREE (analyzed);

>    if (call_uses < 0)

>      return true;

> diff --git a/gcc/testsuite/gcc.dg/ipa/pr95113.c b/gcc/testsuite/gcc.dg/ipa/pr95113.c

> new file mode 100644

> index 00000000000..a8f8c901ebe

> --- /dev/null

> +++ b/gcc/testsuite/gcc.dg/ipa/pr95113.c

> @@ -0,0 +1,33 @@

> +/* { dg-do run } */

> +/* { dg-options "-O2 -fexceptions -fnon-call-exceptions" } */

> +/* { dg-require-effective-target exceptions } */

> +

> +int a, b;

> +

> +static inline long int

> +foo (long int x, int y)

> +{

> +  if (y == 0)

> +    return 0;

> +

> +  if (x == -1 && y == -1)

> +    return 0;

> +

> +  return x / y;

> +}

> +

> +static inline int

> +bar (int *p)

> +{

> +  int c = foo (a, 1) + *p;

> +  return b;

> +}

> +

> +int

> +main ()

> +{

> +  int d = 0;

> +  b = foo (1, 1);

> +  bar (&d);

> +  return 0;

> +}

> diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c

> index 10ef2e3157c..4246dca8806 100644

> --- a/gcc/tree-eh.c

> +++ b/gcc/tree-eh.c

> @@ -2936,6 +2936,16 @@ stmt_could_throw_p (function *fun, gimple *stmt)

>      }

>  }

>  

> +/* Return true if STMT in function FUN must be assumed necessary because of

> +   non-call exceptions.  */

> +

> +bool

> +stmt_unremovable_because_of_non_call_eh_p (function *fun, gimple *stmt)

> +{

> +  return (fun->can_throw_non_call_exceptions

> +	  && !fun->can_delete_dead_exceptions

> +	  && stmt_could_throw_p (fun, stmt));

> +}

>  

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

>  

> diff --git a/gcc/tree-eh.h b/gcc/tree-eh.h

> index cb1019e0d87..ba911cadbe7 100644

> --- a/gcc/tree-eh.h

> +++ b/gcc/tree-eh.h

> @@ -39,6 +39,7 @@ extern bool operation_could_trap_p (enum tree_code, bool, bool, tree);

>  extern bool tree_could_trap_p (tree);

>  extern tree rewrite_to_non_trapping_overflow (tree);

>  extern bool stmt_could_throw_p (function *, gimple *);

> +extern bool stmt_unremovable_because_of_non_call_eh_p (function *, gimple *);

>  extern bool tree_could_throw_p (tree);

>  extern bool stmt_can_throw_external (function *, gimple *);

>  extern bool stmt_can_throw_internal (function *, gimple *);

> diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c

> index 757cfad5b5e..fae5ae72340 100644

> --- a/gcc/tree-ssa-dce.c

> +++ b/gcc/tree-ssa-dce.c

> @@ -201,9 +201,7 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)

>  {

>    /* With non-call exceptions, we have to assume that all statements could

>       throw.  If a statement could throw, it can be deemed necessary.  */

> -  if (cfun->can_throw_non_call_exceptions

> -      && !cfun->can_delete_dead_exceptions

> -      && stmt_could_throw_p (cfun, stmt))

> +  if (stmt_unremovable_because_of_non_call_eh_p (cfun, stmt))

>      {

>        mark_stmt_necessary (stmt, true);

>        return;

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Patch

diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
index 7c922e40a4e..c81e8869e7a 100644
--- a/gcc/ipa-sra.c
+++ b/gcc/ipa-sra.c
@@ -795,17 +795,17 @@  get_single_param_flow_source (const isra_param_flow *param_flow)
 }
 
 /* Inspect all uses of NAME and simple arithmetic calculations involving NAME
-   in NODE and return a negative number if any of them is used for something
-   else than either an actual call argument, simple arithmetic operation or
-   debug statement.  If there are no such uses, return the number of actual
-   arguments that this parameter eventually feeds to (or zero if there is none).
-   For any such parameter, mark PARM_NUM as one of its sources.  ANALYZED is a
-   bitmap that tracks which SSA names we have already started
-   investigating.  */
+   in FUN represented with NODE and return a negative number if any of them is
+   used for something else than either an actual call argument, simple
+   arithmetic operation or debug statement.  If there are no such uses, return
+   the number of actual arguments that this parameter eventually feeds to (or
+   zero if there is none).  For any such parameter, mark PARM_NUM as one of its
+   sources.  ANALYZED is a bitmap that tracks which SSA names we have already
+   started investigating.  */
 
 static int
-isra_track_scalar_value_uses (cgraph_node *node, tree name, int parm_num,
-			      bitmap analyzed)
+isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name,
+			      int parm_num, bitmap analyzed)
 {
   int res = 0;
   imm_use_iterator imm_iter;
@@ -859,8 +859,9 @@  isra_track_scalar_value_uses (cgraph_node *node, tree name, int parm_num,
 	    }
 	  res += all_uses;
 	}
-      else if ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt))
-	       || gimple_code (stmt) == GIMPLE_PHI)
+      else if (!stmt_unremovable_because_of_non_call_eh_p (fun, stmt)
+	       && ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt))
+		   || gimple_code (stmt) == GIMPLE_PHI))
 	{
 	  tree lhs;
 	  if (gimple_code (stmt) == GIMPLE_PHI)
@@ -876,7 +877,7 @@  isra_track_scalar_value_uses (cgraph_node *node, tree name, int parm_num,
 	  gcc_assert (!gimple_vdef (stmt));
 	  if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs)))
 	    {
-	      int tmp = isra_track_scalar_value_uses (node, lhs, parm_num,
+	      int tmp = isra_track_scalar_value_uses (fun, node, lhs, parm_num,
 						      analyzed);
 	      if (tmp < 0)
 		{
@@ -927,7 +928,8 @@  isra_track_scalar_param_local_uses (function *fun, cgraph_node *node, tree parm,
     return true;
 
   bitmap analyzed = BITMAP_ALLOC (NULL);
-  int call_uses = isra_track_scalar_value_uses (node, name, parm_num, analyzed);
+  int call_uses = isra_track_scalar_value_uses (fun, node, name, parm_num,
+						analyzed);
   BITMAP_FREE (analyzed);
   if (call_uses < 0)
     return true;
diff --git a/gcc/testsuite/gcc.dg/ipa/pr95113.c b/gcc/testsuite/gcc.dg/ipa/pr95113.c
new file mode 100644
index 00000000000..a8f8c901ebe
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr95113.c
@@ -0,0 +1,33 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -fexceptions -fnon-call-exceptions" } */
+/* { dg-require-effective-target exceptions } */
+
+int a, b;
+
+static inline long int
+foo (long int x, int y)
+{
+  if (y == 0)
+    return 0;
+
+  if (x == -1 && y == -1)
+    return 0;
+
+  return x / y;
+}
+
+static inline int
+bar (int *p)
+{
+  int c = foo (a, 1) + *p;
+  return b;
+}
+
+int
+main ()
+{
+  int d = 0;
+  b = foo (1, 1);
+  bar (&d);
+  return 0;
+}
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 10ef2e3157c..4246dca8806 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2936,6 +2936,16 @@  stmt_could_throw_p (function *fun, gimple *stmt)
     }
 }
 
+/* Return true if STMT in function FUN must be assumed necessary because of
+   non-call exceptions.  */
+
+bool
+stmt_unremovable_because_of_non_call_eh_p (function *fun, gimple *stmt)
+{
+  return (fun->can_throw_non_call_exceptions
+	  && !fun->can_delete_dead_exceptions
+	  && stmt_could_throw_p (fun, stmt));
+}
 
 /* Return true if expression T could throw an exception.  */
 
diff --git a/gcc/tree-eh.h b/gcc/tree-eh.h
index cb1019e0d87..ba911cadbe7 100644
--- a/gcc/tree-eh.h
+++ b/gcc/tree-eh.h
@@ -39,6 +39,7 @@  extern bool operation_could_trap_p (enum tree_code, bool, bool, tree);
 extern bool tree_could_trap_p (tree);
 extern tree rewrite_to_non_trapping_overflow (tree);
 extern bool stmt_could_throw_p (function *, gimple *);
+extern bool stmt_unremovable_because_of_non_call_eh_p (function *, gimple *);
 extern bool tree_could_throw_p (tree);
 extern bool stmt_can_throw_external (function *, gimple *);
 extern bool stmt_can_throw_internal (function *, gimple *);
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index 757cfad5b5e..fae5ae72340 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -201,9 +201,7 @@  mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
 {
   /* With non-call exceptions, we have to assume that all statements could
      throw.  If a statement could throw, it can be deemed necessary.  */
-  if (cfun->can_throw_non_call_exceptions
-      && !cfun->can_delete_dead_exceptions
-      && stmt_could_throw_p (cfun, stmt))
+  if (stmt_unremovable_because_of_non_call_eh_p (cfun, stmt))
     {
       mark_stmt_necessary (stmt, true);
       return;