diff mbox

Introduce can_remove_lhs_p

Message ID 20160523154416.GT1611@redhat.com
State New
Headers show

Commit Message

Marek Polacek May 23, 2016, 3:44 p.m. UTC
On Mon, May 23, 2016 at 04:36:30PM +0200, Jakub Jelinek wrote:
> On Mon, May 23, 2016 at 04:28:33PM +0200, Marek Polacek wrote:
> > As promised in <https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01638.html>,
> > this is a simple clean-up which makes use of a new predicate.  Richi suggested
> > adding maybe_drop_lhs_from_noreturn_call which would be nicer, but I didn't
> > know how to do that, given the handling if lhs is an SSA_NAME.
> 
> Shouldn't it be should_remove_lhs_p instead?
> I mean, it is not just an optimization, but part of how we define the IL.
 
Aha, ok.  Renamed.

> Shouldn't it be also used in tree-cfg.c (verify_gimple_call)?

I left that spot on purpose but now I don't quite see why, fixed.  Thanks,

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2016-05-23  Marek Polacek  <polacek@redhat.com>

	* tree.h (should_remove_lhs_p): New predicate.
	* cgraph.c (cgraph_edge::redirect_call_stmt_to_callee): Use it.
	* gimple-fold.c (gimple_fold_call): Likewise.
	* gimplify.c (gimplify_modify_expr): Likewise.
	* tree-cfg.c (verify_gimple_call): Likewise.
	* tree-cfgcleanup.c (fixup_noreturn_call): Likewise.


	Marek

Comments

Richard Biener May 24, 2016, 12:17 p.m. UTC | #1
On Mon, 23 May 2016, Marek Polacek wrote:

> On Mon, May 23, 2016 at 04:36:30PM +0200, Jakub Jelinek wrote:
> > On Mon, May 23, 2016 at 04:28:33PM +0200, Marek Polacek wrote:
> > > As promised in <https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01638.html>,
> > > this is a simple clean-up which makes use of a new predicate.  Richi suggested
> > > adding maybe_drop_lhs_from_noreturn_call which would be nicer, but I didn't
> > > know how to do that, given the handling if lhs is an SSA_NAME.
> > 
> > Shouldn't it be should_remove_lhs_p instead?
> > I mean, it is not just an optimization, but part of how we define the IL.
>  
> Aha, ok.  Renamed.
> 
> > Shouldn't it be also used in tree-cfg.c (verify_gimple_call)?
> 
> I left that spot on purpose but now I don't quite see why, fixed.  Thanks,
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

Can you move should_remove_lhs_p to tree-cfg.h please?

Ok with that change.

Richard.

> 2016-05-23  Marek Polacek  <polacek@redhat.com>
> 
> 	* tree.h (should_remove_lhs_p): New predicate.
> 	* cgraph.c (cgraph_edge::redirect_call_stmt_to_callee): Use it.
> 	* gimple-fold.c (gimple_fold_call): Likewise.
> 	* gimplify.c (gimplify_modify_expr): Likewise.
> 	* tree-cfg.c (verify_gimple_call): Likewise.
> 	* tree-cfgcleanup.c (fixup_noreturn_call): Likewise.
> 
> diff --git gcc/cgraph.c gcc/cgraph.c
> index cf9192f..1a4f665 100644
> --- gcc/cgraph.c
> +++ gcc/cgraph.c
> @@ -1513,10 +1513,7 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
>      }
>  
>    /* If the call becomes noreturn, remove the LHS if possible.  */
> -  if (lhs
> -      && (gimple_call_flags (new_stmt) & ECF_NORETURN)
> -      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
> -      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
> +  if (gimple_call_noreturn_p (new_stmt) && should_remove_lhs_p (lhs))
>      {
>        if (TREE_CODE (lhs) == SSA_NAME)
>  	{
> diff --git gcc/gimple-fold.c gcc/gimple-fold.c
> index 858f484..6b50d43 100644
> --- gcc/gimple-fold.c
> +++ gcc/gimple-fold.c
> @@ -3052,12 +3052,9 @@ gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace)
>  			  == void_type_node))
>  		    gimple_call_set_fntype (stmt, TREE_TYPE (fndecl));
>  		  /* If the call becomes noreturn, remove the lhs.  */
> -		  if (lhs
> -		      && (gimple_call_flags (stmt) & ECF_NORETURN)
> +		  if (gimple_call_noreturn_p (stmt)
>  		      && (VOID_TYPE_P (TREE_TYPE (gimple_call_fntype (stmt)))
> -			  || ((TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs)))
> -			       == INTEGER_CST)
> -			      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))))
> +			  || should_remove_lhs_p (lhs)))
>  		    {
>  		      if (TREE_CODE (lhs) == SSA_NAME)
>  			{
> diff --git gcc/gimplify.c gcc/gimplify.c
> index 4a544e3..c77eb51 100644
> --- gcc/gimplify.c
> +++ gcc/gimplify.c
> @@ -4847,9 +4847,7 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>  	    }
>  	}
>        notice_special_calls (call_stmt);
> -      if (!gimple_call_noreturn_p (call_stmt)
> -	  || TREE_ADDRESSABLE (TREE_TYPE (*to_p))
> -	  || TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (*to_p))) != INTEGER_CST)
> +      if (!gimple_call_noreturn_p (call_stmt) || !should_remove_lhs_p (*to_p))
>  	gimple_call_set_lhs (call_stmt, *to_p);
>        else if (TREE_CODE (*to_p) == SSA_NAME)
>  	/* The above is somewhat premature, avoid ICEing later for a
> diff --git gcc/tree-cfg.c gcc/tree-cfg.c
> index 7c2ee78..82f0da6c 100644
> --- gcc/tree-cfg.c
> +++ gcc/tree-cfg.c
> @@ -3385,11 +3385,9 @@ verify_gimple_call (gcall *stmt)
>        return true;
>      }
>  
> -  if (lhs
> -      && gimple_call_ctrl_altering_p (stmt)
> +  if (gimple_call_ctrl_altering_p (stmt)
>        && gimple_call_noreturn_p (stmt)
> -      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
> -      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
> +      && should_remove_lhs_p (lhs))
>      {
>        error ("LHS in noreturn call");
>        return true;
> diff --git gcc/tree-cfgcleanup.c gcc/tree-cfgcleanup.c
> index 46d0fa3..4134c38 100644
> --- gcc/tree-cfgcleanup.c
> +++ gcc/tree-cfgcleanup.c
> @@ -604,8 +604,7 @@ fixup_noreturn_call (gimple *stmt)
>       temporaries of variable-sized types is not supported.  Also don't
>       do this with TREE_ADDRESSABLE types, as assign_temp will abort.  */
>    tree lhs = gimple_call_lhs (stmt);
> -  if (lhs && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
> -      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
> +  if (should_remove_lhs_p (lhs))
>      {
>        gimple_call_set_lhs (stmt, NULL_TREE);
>  
> diff --git gcc/tree.h gcc/tree.h
> index 2510d16..1d72437 100644
> --- gcc/tree.h
> +++ gcc/tree.h
> @@ -5471,4 +5471,14 @@ desired_pro_or_demotion_p (const_tree to_type, const_tree from_type)
>    return to_type_precision <= TYPE_PRECISION (from_type);
>  }
>  
> +/* Return true if the LHS of a call should be removed.  */
> +
> +inline bool
> +should_remove_lhs_p (tree lhs)
> +{
> +  return (lhs
> +	  && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
> +	  && !TREE_ADDRESSABLE (TREE_TYPE (lhs)));
> +}
> +
>  #endif  /* GCC_TREE_H  */
> 
> 	Marek
> 
>
diff mbox

Patch

diff --git gcc/cgraph.c gcc/cgraph.c
index cf9192f..1a4f665 100644
--- gcc/cgraph.c
+++ gcc/cgraph.c
@@ -1513,10 +1513,7 @@  cgraph_edge::redirect_call_stmt_to_callee (void)
     }
 
   /* If the call becomes noreturn, remove the LHS if possible.  */
-  if (lhs
-      && (gimple_call_flags (new_stmt) & ECF_NORETURN)
-      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
-      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
+  if (gimple_call_noreturn_p (new_stmt) && should_remove_lhs_p (lhs))
     {
       if (TREE_CODE (lhs) == SSA_NAME)
 	{
diff --git gcc/gimple-fold.c gcc/gimple-fold.c
index 858f484..6b50d43 100644
--- gcc/gimple-fold.c
+++ gcc/gimple-fold.c
@@ -3052,12 +3052,9 @@  gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace)
 			  == void_type_node))
 		    gimple_call_set_fntype (stmt, TREE_TYPE (fndecl));
 		  /* If the call becomes noreturn, remove the lhs.  */
-		  if (lhs
-		      && (gimple_call_flags (stmt) & ECF_NORETURN)
+		  if (gimple_call_noreturn_p (stmt)
 		      && (VOID_TYPE_P (TREE_TYPE (gimple_call_fntype (stmt)))
-			  || ((TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs)))
-			       == INTEGER_CST)
-			      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))))
+			  || should_remove_lhs_p (lhs)))
 		    {
 		      if (TREE_CODE (lhs) == SSA_NAME)
 			{
diff --git gcc/gimplify.c gcc/gimplify.c
index 4a544e3..c77eb51 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -4847,9 +4847,7 @@  gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    }
 	}
       notice_special_calls (call_stmt);
-      if (!gimple_call_noreturn_p (call_stmt)
-	  || TREE_ADDRESSABLE (TREE_TYPE (*to_p))
-	  || TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (*to_p))) != INTEGER_CST)
+      if (!gimple_call_noreturn_p (call_stmt) || !should_remove_lhs_p (*to_p))
 	gimple_call_set_lhs (call_stmt, *to_p);
       else if (TREE_CODE (*to_p) == SSA_NAME)
 	/* The above is somewhat premature, avoid ICEing later for a
diff --git gcc/tree-cfg.c gcc/tree-cfg.c
index 7c2ee78..82f0da6c 100644
--- gcc/tree-cfg.c
+++ gcc/tree-cfg.c
@@ -3385,11 +3385,9 @@  verify_gimple_call (gcall *stmt)
       return true;
     }
 
-  if (lhs
-      && gimple_call_ctrl_altering_p (stmt)
+  if (gimple_call_ctrl_altering_p (stmt)
       && gimple_call_noreturn_p (stmt)
-      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
-      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
+      && should_remove_lhs_p (lhs))
     {
       error ("LHS in noreturn call");
       return true;
diff --git gcc/tree-cfgcleanup.c gcc/tree-cfgcleanup.c
index 46d0fa3..4134c38 100644
--- gcc/tree-cfgcleanup.c
+++ gcc/tree-cfgcleanup.c
@@ -604,8 +604,7 @@  fixup_noreturn_call (gimple *stmt)
      temporaries of variable-sized types is not supported.  Also don't
      do this with TREE_ADDRESSABLE types, as assign_temp will abort.  */
   tree lhs = gimple_call_lhs (stmt);
-  if (lhs && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
-      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
+  if (should_remove_lhs_p (lhs))
     {
       gimple_call_set_lhs (stmt, NULL_TREE);
 
diff --git gcc/tree.h gcc/tree.h
index 2510d16..1d72437 100644
--- gcc/tree.h
+++ gcc/tree.h
@@ -5471,4 +5471,14 @@  desired_pro_or_demotion_p (const_tree to_type, const_tree from_type)
   return to_type_precision <= TYPE_PRECISION (from_type);
 }
 
+/* Return true if the LHS of a call should be removed.  */
+
+inline bool
+should_remove_lhs_p (tree lhs)
+{
+  return (lhs
+	  && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
+	  && !TREE_ADDRESSABLE (TREE_TYPE (lhs)));
+}
+
 #endif  /* GCC_TREE_H  */