[PATCH] middle-end: fix de-optimizations with bitclear patterns on signed values

Tamar Christina Tamar.Christina@arm.com
Fri Nov 12 07:30:35 GMT 2021



> -----Original Message-----
> From: Jakub Jelinek <jakub@redhat.com>
> Sent: Thursday, November 4, 2021 4:11 PM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: Jonathan Wakely <jwakely@redhat.com>; Richard Biener
> <rguenther@suse.de>; gcc-patches@gcc.gnu.org; nd <nd@arm.com>
> Subject: Re: [PATCH] middle-end: fix de-optimizations with bitclear patterns
> on signed values
> 
> On Thu, Nov 04, 2021 at 12:19:34PM +0000, Tamar Christina wrote:
> > I'm not sure the precision matters since if the conversion resulted in
> > not enough precision such that It influences the compare it would have
> been optimized out.
> 
> You can't really rely on other optimizations being performed.  They will
> usually happen, but might not because such code only materialized short
> time ago without folding happening in between, or some debug counters or -
> fno-* disabling some passes, ...

Fair point, I have separated out the logic as you requested and added the debug fix.

Bootstrapped Regtested on aarch64-none-linux-gnu,
x86_64-pc-linux-gnu and no regressions.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	* tree-ssa-phiopt.c (spaceship_replacement): Handle new canonical
	codegen.

--- inline copy of patch ---

diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index 0e339c46afa29fa97f90d9bc4394370cd9b4b396..3ad5b23885a37eec0beff229e2a96e86658b2d1a 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -2038,11 +2038,36 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
   gimple *orig_use_stmt = use_stmt;
   tree orig_use_lhs = NULL_TREE;
   int prec = TYPE_PRECISION (TREE_TYPE (phires));
-  if (is_gimple_assign (use_stmt)
-      && gimple_assign_rhs_code (use_stmt) == BIT_AND_EXPR
-      && TREE_CODE (gimple_assign_rhs2 (use_stmt)) == INTEGER_CST
-      && (wi::to_wide (gimple_assign_rhs2 (use_stmt))
-	  == wi::shifted_mask (1, prec - 1, false, prec)))
+  bool is_cast = false;
+
+  /* Deal with the case when match.pd has rewritten the (res & ~1) == 0
+     into res <= 1 and has left a type-cast for signed types.  */
+  if (gimple_assign_cast_p (use_stmt))
+    {
+      orig_use_lhs = gimple_assign_lhs (use_stmt);
+      /* match.pd would have only done this for a signed type,
+	 so the conversion must be to an unsigned one.  */
+      tree ty1 = TREE_TYPE (gimple_assign_rhs1 (use_stmt));
+      tree ty2 = TREE_TYPE (orig_use_lhs);
+
+      if (!TYPE_UNSIGNED (ty2) || !INTEGRAL_TYPE_P (ty2))
+	return false;
+      if (TYPE_PRECISION (ty1) != TYPE_PRECISION (ty2))
+	return false;
+      if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (orig_use_lhs))
+	return false;
+      if (EDGE_COUNT (phi_bb->preds) != 4)
+	return false;
+      if (!single_imm_use (orig_use_lhs, &use_p, &use_stmt))
+	return false;
+
+      is_cast = true;
+    }
+  else if (is_gimple_assign (use_stmt)
+	   && gimple_assign_rhs_code (use_stmt) == BIT_AND_EXPR
+	   && TREE_CODE (gimple_assign_rhs2 (use_stmt)) == INTEGER_CST
+	   && (wi::to_wide (gimple_assign_rhs2 (use_stmt))
+	       == wi::shifted_mask (1, prec - 1, false, prec)))
     {
       /* For partial_ordering result operator>= with unspec as second
 	 argument is (res & 1) == res, folded by match.pd into
@@ -2099,7 +2124,7 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
       || !tree_fits_shwi_p (rhs)
       || !IN_RANGE (tree_to_shwi (rhs), -1, 1))
     return false;
-  if (orig_use_lhs)
+  if (orig_use_lhs && !is_cast)
     {
       if ((cmp != EQ_EXPR && cmp != NE_EXPR) || !integer_zerop (rhs))
 	return false;
@@ -2310,62 +2335,101 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
     one_cmp = GT_EXPR;
 
   enum tree_code res_cmp;
-  switch (cmp)
+
+  if (is_cast)
     {
-    case EQ_EXPR:
-      if (integer_zerop (rhs))
-	res_cmp = EQ_EXPR;
-      else if (integer_minus_onep (rhs))
-	res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
-      else if (integer_onep (rhs))
-	res_cmp = one_cmp;
-      else
+      if (TREE_CODE (rhs) != INTEGER_CST)
 	return false;
-      break;
-    case NE_EXPR:
-      if (integer_zerop (rhs))
-	res_cmp = NE_EXPR;
-      else if (integer_minus_onep (rhs))
-	res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
-      else if (integer_onep (rhs))
-	res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
-      else
-	return false;
-      break;
-    case LT_EXPR:
-      if (integer_onep (rhs))
-	res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
-      else if (integer_zerop (rhs))
-	res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
-      else
-	return false;
-      break;
-    case LE_EXPR:
-      if (integer_zerop (rhs))
-	res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
-      else if (integer_minus_onep (rhs))
-	res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
-      else
-	return false;
-      break;
-    case GT_EXPR:
-      if (integer_minus_onep (rhs))
-	res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
-      else if (integer_zerop (rhs))
-	res_cmp = one_cmp;
-      else
-	return false;
-      break;
-    case GE_EXPR:
-      if (integer_zerop (rhs))
-	res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
-      else if (integer_onep (rhs))
-	res_cmp = one_cmp;
-      else
-	return false;
-      break;
-    default:
-      gcc_unreachable ();
+      /* As for -ffast-math we assume the 2 return to be
+	 impossible, canonicalize (unsigned) res <= 1U or
+	 (unsigned) res < 2U into res >= 0 and (unsigned) res > 1U
+	 or (unsigned) res >= 2U as res < 0.  */
+      switch (cmp)
+	{
+	case LE_EXPR:
+	  if (!integer_onep (rhs))
+	    return false;
+	  res_cmp = GE_EXPR;
+	  break;
+	case LT_EXPR:
+	  if (wi::ne_p (wi::to_widest (rhs), 2))
+	    return false;
+	  res_cmp = GE_EXPR;
+	  break;
+	case GT_EXPR:
+	  if (!integer_onep (rhs))
+	    return false;
+	  res_cmp = LT_EXPR;
+	  break;
+	case GE_EXPR:
+	  if (wi::ne_p (wi::to_widest (rhs), 2))
+	    return false;
+	  res_cmp = LT_EXPR;
+	  break;
+	default:
+	  return false;
+	}
+      rhs = build_zero_cst (TREE_TYPE (phires));
+    }
+  else
+    {
+      switch (cmp)
+      {
+      case EQ_EXPR:
+	if (integer_zerop (rhs))
+	  res_cmp = EQ_EXPR;
+	else if (integer_minus_onep (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
+	else if (integer_onep (rhs))
+	  res_cmp = one_cmp;
+	else
+	  return false;
+	break;
+      case NE_EXPR:
+	if (integer_zerop (rhs))
+	  res_cmp = NE_EXPR;
+	else if (integer_minus_onep (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
+	else if (integer_onep (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
+	else
+	  return false;
+	break;
+      case LT_EXPR:
+	if (integer_onep (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
+	else if (integer_zerop (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
+	else
+	  return false;
+	break;
+      case LE_EXPR:
+	if (integer_zerop (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
+	else if (integer_minus_onep (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
+	else
+	  return false;
+	break;
+      case GT_EXPR:
+	if (integer_minus_onep (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
+	else if (integer_zerop (rhs))
+	  res_cmp = one_cmp;
+	else
+	  return false;
+	break;
+      case GE_EXPR:
+	if (integer_zerop (rhs))
+	  res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
+	else if (integer_onep (rhs))
+	  res_cmp = one_cmp;
+	else
+	  return false;
+	break;
+      default:
+	  gcc_unreachable ();
+      }
     }
 
   if (gimple_code (use_stmt) == GIMPLE_COND)
@@ -2394,6 +2458,7 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
       use_operand_p use_p;
       imm_use_iterator iter;
       bool has_debug_uses = false;
+      bool has_cast_debug_uses = false;
       FOR_EACH_IMM_USE_FAST (use_p, iter, phires)
 	{
 	  gimple *use_stmt = USE_STMT (use_p);
@@ -2405,12 +2470,14 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
 	}
       if (orig_use_lhs)
 	{
-	  if (!has_debug_uses)
+	  if (!has_debug_uses || is_cast)
 	    FOR_EACH_IMM_USE_FAST (use_p, iter, orig_use_lhs)
 	      {
 		gimple *use_stmt = USE_STMT (use_p);
 		gcc_assert (is_gimple_debug (use_stmt));
 		has_debug_uses = true;
+		if (is_cast)
+		  has_cast_debug_uses = true;
 	      }
 	  gimple_stmt_iterator gsi = gsi_for_stmt (orig_use_stmt);
 	  tree zero = build_zero_cst (TREE_TYPE (orig_use_lhs));
@@ -2448,7 +2515,23 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
 	  gsi_insert_before (&gsi, g, GSI_SAME_STMT);
 	  replace_uses_by (phires, temp2);
 	  if (orig_use_lhs)
-	    replace_uses_by (orig_use_lhs, temp2);
+	    {
+	      if (has_cast_debug_uses)
+		{
+		  tree temp3 = make_node (DEBUG_EXPR_DECL);
+		  DECL_ARTIFICIAL (temp3) = 1;
+		  TREE_TYPE (temp3) = TREE_TYPE (orig_use_lhs);
+		  SET_DECL_MODE (temp3, TYPE_MODE (type));
+		  t = build2 (EQ_EXPR, boolean_type_node, lhs1, rhs2);
+		  t = build3 (COND_EXPR, type, t, build_zero_cst (type),
+			      temp1);
+		  g = gimple_build_debug_bind (temp3, t, phi);
+		  gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+		  replace_uses_by (orig_use_lhs, temp3);
+		}
+	      else
+		replace_uses_by (orig_use_lhs, temp2);
+	    }
 	}
     }

-------------- next part --------------
A non-text attachment was scrubbed...
Name: rb14938.patch
Type: application/octet-stream
Size: 7989 bytes
Desc: rb14938.patch
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20211112/ac1ac77d/attachment-0001.obj>


More information about the Gcc-patches mailing list