[1/2] PHIOPT: Allow moving of some builtin calls

Message ID 20230428170213.677572-2-apinski@marvell.com
State Accepted
Headers
Series Porting of builtin_zero_pattern to match |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski April 28, 2023, 5:02 p.m. UTC
  While moving working on moving
cond_removal_in_builtin_zero_pattern to match, I noticed
that functions were not allowed to move as we reject all
non-assignments.
This changes to allowing a few calls which are known not
to throw/trap. Right now it is restricted to ones
which cond_removal_in_builtin_zero_pattern handles but
adding more is just adding it to the switch statement.

gcc/ChangeLog:

	* tree-ssa-phiopt.cc (empty_bb_or_one_feeding_into_p):
	Allow some builtin/internal function calls which
	are known not to trap/throw.
	(phiopt_worker::match_simplify_replacement):
	Use name instead of getting the lhs again.
---
 gcc/tree-ssa-phiopt.cc | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)
  

Comments

Jeff Law April 30, 2023, 7:11 p.m. UTC | #1
On 4/28/23 11:02, Andrew Pinski via Gcc-patches wrote:
> While moving working on moving
> cond_removal_in_builtin_zero_pattern to match, I noticed
> that functions were not allowed to move as we reject all
> non-assignments.
> This changes to allowing a few calls which are known not
> to throw/trap. Right now it is restricted to ones
> which cond_removal_in_builtin_zero_pattern handles but
> adding more is just adding it to the switch statement.
> 
> gcc/ChangeLog:
> 
> 	* tree-ssa-phiopt.cc (empty_bb_or_one_feeding_into_p):
> 	Allow some builtin/internal function calls which
> 	are known not to trap/throw.
> 	(phiopt_worker::match_simplify_replacement):
> 	Use name instead of getting the lhs again.
OK
jeff
  

Patch

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 4b43f1abdbc..024a4362093 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -548,6 +548,7 @@  empty_bb_or_one_feeding_into_p (basic_block bb,
 {
   stmt = nullptr;
   gimple *stmt_to_move = nullptr;
+  tree lhs;
 
   if (empty_block_p (bb))
     return true;
@@ -592,17 +593,43 @@  empty_bb_or_one_feeding_into_p (basic_block bb,
   if (gimple_uses_undefined_value_p (stmt_to_move))
     return false;
 
-  /* Allow assignments and not no calls.
+  /* Allow assignments but allow some builtin/internal calls.
      As const calls don't match any of the above, yet they could
      still have some side-effects - they could contain
      gimple_could_trap_p statements, like floating point
      exceptions or integer division by zero.  See PR70586.
      FIXME: perhaps gimple_has_side_effects or gimple_could_trap_p
-     should handle this.  */
+     should handle this.
+     Allow some known builtin/internal calls that are known not to
+     trap: logical functions (e.g. bswap and bit counting). */
   if (!is_gimple_assign (stmt_to_move))
-    return false;
+    {
+      if (!is_gimple_call (stmt_to_move))
+	return false;
+      combined_fn cfn = gimple_call_combined_fn (stmt_to_move);
+      switch (cfn)
+	{
+	default:
+	  return false;
+	case CFN_BUILT_IN_BSWAP16:
+	case CFN_BUILT_IN_BSWAP32:
+	case CFN_BUILT_IN_BSWAP64:
+	case CFN_BUILT_IN_BSWAP128:
+	CASE_CFN_FFS:
+	CASE_CFN_PARITY:
+	CASE_CFN_POPCOUNT:
+	CASE_CFN_CLZ:
+	CASE_CFN_CTZ:
+	case CFN_BUILT_IN_CLRSB:
+	case CFN_BUILT_IN_CLRSBL:
+	case CFN_BUILT_IN_CLRSBLL:
+	  lhs = gimple_call_lhs (stmt_to_move);
+	  break;
+	}
+    }
+  else
+    lhs = gimple_assign_lhs (stmt_to_move);
 
-  tree lhs = gimple_assign_lhs (stmt_to_move);
   gimple *use_stmt;
   use_operand_p use_p;