middle-end: PR tree-optimization/21137: STRIP_NOPS avoids missed optimization.

Message ID 001201d677db$e0869d70$a193d850$@nextmovesoftware.com
State New
Headers show
Series
  • middle-end: PR tree-optimization/21137: STRIP_NOPS avoids missed optimization.
Related show

Commit Message

Roger Sayle Aug. 21, 2020, 4:55 p.m.
PR tree-optimization/21137 is now an old enhancement request pointing out
that an optimization I added back in 2006, to optimize "((x>>31)&64) != 0"
as "x < 0", doesn't fire in the presence of unanticipated type conversions.
The fix is to call STRIP_NOPS at the appropriate point.

I'd considered moving this transformation to match.pd, but it's a lot of
complex logic that (I suspect) would be just as ugly in match.pd as it is
in fold-const.c.

This patch has been tested on x86_64-pc-linux-gnu with a "make bootstrap"
and "make -k check" with no new failures.
Ok for mainline?

2020-08-21  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	PR tree-optimization/21137
	* gcc/fold-const.c (fold_binary_loc) [NE_EXPR/EQ_EXPR]: Call
	STRIP_NOPS when checking whether to simplify ((x>>C1)&C2) != 0.

gcc/testsuite/ChangeLog
	PR tree-optimization/21137
	* gcc.dg/pr21137.c: New test.


Thanks in advance,
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK

diff --git a/gcc/testsuite/gcc.dg/pr21137.c b/gcc/testsuite/gcc.dg/pr21137.c
new file mode 100644
index 0000000..6d73dea
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr21137.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+void foo();
+
+void test5_1(int e)
+{
+  if ((e >> 31) & 64)
+    foo();
+}
+
+typedef int myint;
+
+void test5_2(myint e)
+{
+  if ((e >> 31) & 64)
+    foo();
+}
+
+/* { dg-final { scan-tree-dump-times " < 0" 2 "optimized" } } */

Comments

Jakub Jelinek via Gcc-patches Aug. 24, 2020, 11:11 p.m. | #1
On Fri, 2020-08-21 at 17:55 +0100, Roger Sayle wrote:
> PR tree-optimization/21137 is now an old enhancement request pointing out

> that an optimization I added back in 2006, to optimize "((x>>31)&64) != 0"

> as "x < 0", doesn't fire in the presence of unanticipated type conversions.

> The fix is to call STRIP_NOPS at the appropriate point.

> 

> I'd considered moving this transformation to match.pd, but it's a lot of

> complex logic that (I suspect) would be just as ugly in match.pd as it is

> in fold-const.c.

> 

> This patch has been tested on x86_64-pc-linux-gnu with a "make bootstrap"

> and "make -k check" with no new failures.

> Ok for mainline?

> 

> 2020-08-21  Roger Sayle  <roger@nextmovesoftware.com>

> 

> gcc/ChangeLog

> 	PR tree-optimization/21137

> 	* gcc/fold-const.c (fold_binary_loc) [NE_EXPR/EQ_EXPR]: Call

> 	STRIP_NOPS when checking whether to simplify ((x>>C1)&C2) != 0.

> 

> gcc/testsuite/ChangeLog

> 	PR tree-optimization/21137

> 	* gcc.dg/pr21137.c: New test.

OK
jeff
>

Patch

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 9fc4c2a..efe77e7 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -11596,45 +11596,53 @@  fold_binary_loc (location_t loc, enum tree_code code, tree type,
 	 C1 is a valid shift constant, and C2 is a power of two, i.e.
 	 a single bit.  */
       if (TREE_CODE (arg0) == BIT_AND_EXPR
-	  && TREE_CODE (TREE_OPERAND (arg0, 0)) == RSHIFT_EXPR
-	  && TREE_CODE (TREE_OPERAND (TREE_OPERAND (arg0, 0), 1))
-	     == INTEGER_CST
 	  && integer_pow2p (TREE_OPERAND (arg0, 1))
 	  && integer_zerop (arg1))
 	{
-	  tree itype = TREE_TYPE (arg0);
-	  tree arg001 = TREE_OPERAND (TREE_OPERAND (arg0, 0), 1);
-	  prec = TYPE_PRECISION (itype);
-
-	  /* Check for a valid shift count.  */
-	  if (wi::ltu_p (wi::to_wide (arg001), prec))
+	  tree arg00 = TREE_OPERAND (arg0, 0);
+	  STRIP_NOPS (arg00);
+	  if (TREE_CODE (arg00) == RSHIFT_EXPR
+	      && TREE_CODE (TREE_OPERAND (arg00, 1)) == INTEGER_CST)
 	    {
-	      tree arg01 = TREE_OPERAND (arg0, 1);
-	      tree arg000 = TREE_OPERAND (TREE_OPERAND (arg0, 0), 0);
-	      unsigned HOST_WIDE_INT log2 = tree_log2 (arg01);
-	      /* If (C2 << C1) doesn't overflow, then ((X >> C1) & C2) != 0
-		 can be rewritten as (X & (C2 << C1)) != 0.  */
-	      if ((log2 + TREE_INT_CST_LOW (arg001)) < prec)
+	      tree itype = TREE_TYPE (arg00);
+	      tree arg001 = TREE_OPERAND (arg00, 1);
+	      prec = TYPE_PRECISION (itype);
+
+	      /* Check for a valid shift count.  */
+	      if (wi::ltu_p (wi::to_wide (arg001), prec))
 		{
-		  tem = fold_build2_loc (loc, LSHIFT_EXPR, itype, arg01, arg001);
-		  tem = fold_build2_loc (loc, BIT_AND_EXPR, itype, arg000, tem);
-		  return fold_build2_loc (loc, code, type, tem,
-					  fold_convert_loc (loc, itype, arg1));
-		}
-	      /* Otherwise, for signed (arithmetic) shifts,
-		 ((X >> C1) & C2) != 0 is rewritten as X < 0, and
-		 ((X >> C1) & C2) == 0 is rewritten as X >= 0.  */
-	      else if (!TYPE_UNSIGNED (itype))
-		return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR, type,
-				    arg000, build_int_cst (itype, 0));
-	      /* Otherwise, of unsigned (logical) shifts,
-		 ((X >> C1) & C2) != 0 is rewritten as (X,false), and
-		 ((X >> C1) & C2) == 0 is rewritten as (X,true).  */
-	      else
-		return omit_one_operand_loc (loc, type,
+		  tree arg01 = TREE_OPERAND (arg0, 1);
+		  tree arg000 = TREE_OPERAND (arg00, 0);
+		  unsigned HOST_WIDE_INT log2 = tree_log2 (arg01);
+		  /* If (C2 << C1) doesn't overflow, then
+		     ((X >> C1) & C2) != 0 can be rewritten as
+		     (X & (C2 << C1)) != 0.  */
+		  if ((log2 + TREE_INT_CST_LOW (arg001)) < prec)
+		    {
+		      tem = fold_build2_loc (loc, LSHIFT_EXPR, itype,
+					     arg01, arg001);
+		      tem = fold_build2_loc (loc, BIT_AND_EXPR, itype,
+					     arg000, tem);
+		      return fold_build2_loc (loc, code, type, tem,
+				fold_convert_loc (loc, itype, arg1));
+		    }
+		  /* Otherwise, for signed (arithmetic) shifts,
+		     ((X >> C1) & C2) != 0 is rewritten as X < 0, and
+		     ((X >> C1) & C2) == 0 is rewritten as X >= 0.  */
+		  else if (!TYPE_UNSIGNED (itype))
+		    return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR
+								 : LT_EXPR,
+					    type, arg000,
+					    build_int_cst (itype, 0));
+		  /* Otherwise, of unsigned (logical) shifts,
+		     ((X >> C1) & C2) != 0 is rewritten as (X,false), and
+		     ((X >> C1) & C2) == 0 is rewritten as (X,true).  */
+		  else
+		    return omit_one_operand_loc (loc, type,
 					 code == EQ_EXPR ? integer_one_node
 							 : integer_zero_node,
 					 arg000);
+		}
 	    }
 	}