[PATCHv2 1/2] Improve do_store_flag for single bit comparison against 0

[PATCHv2 1/2] Improve do_store_flag for single bit comparison against 0

[Andrew Pinski]

While working something else, I noticed we could improve
the following function code generation:
```
unsigned f(unsigned t)
{
  if (t & ~(1<<30)) __builtin_unreachable();
  return t != 0;
}
```
Right know we just emit a comparison against 0 instead
of just a shift right by 30.
There is code in do_store_flag which already optimizes
`(t & 1<<30) != 0` to `(t >> 30) & 1` (using bit extraction if available).
This patch extends it to handle the case where we know t has a nonzero
of just one bit set.

Changes from v1:
* v2: Updated for the bit extraction improvements.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* expr.cc (do_store_flag): Extend the one bit checking case
	to handle the case where we don't have an and but rather still
	one bit is known to be non-zero.
---
 gcc/expr.cc | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index 6849c9627d0..b85e963b57e 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -13155,16 +13155,31 @@ do_store_flag (sepops ops, rtx target, machine_mode mode)
       && integer_zerop (arg1)
       && (TYPE_PRECISION (ops->type) != 1 || TYPE_UNSIGNED (ops->type)))
     {
-      gimple *srcstmt = get_def_for_expr (arg0, BIT_AND_EXPR);
-      if (srcstmt
-	  && integer_pow2p (gimple_assign_rhs2 (srcstmt)))
+      wide_int nz = tree_nonzero_bits (arg0);
+
+      if (wi::popcount (nz) == 1)
 	{
+	  tree op0;
+	  int bitnum;
+	  gimple *srcstmt = get_def_for_expr (arg0, BIT_AND_EXPR);
+	  /* If the defining statement was (x & POW2), then remove the and
+	     as we are going to add it back. */
+	  if (srcstmt
+	      && integer_pow2p (gimple_assign_rhs2 (srcstmt)))
+	    {
+	      op0 = gimple_assign_rhs1 (srcstmt);
+	      bitnum = tree_log2 (gimple_assign_rhs2 (srcstmt));
+	    }
+	  else
+	    {
+	      op0 = arg0;
+	      bitnum = wi::exact_log2 (nz);
+	    }
 	  enum tree_code tcode = code == NE ? NE_EXPR : EQ_EXPR;
-	  int bitnum = tree_log2 (gimple_assign_rhs2 (srcstmt));
 
 	  type = lang_hooks.types.type_for_mode (mode, unsignedp);
 	  return expand_single_bit_test (loc, tcode,
-					 gimple_assign_rhs1 (srcstmt),
+					 op0,
 					 bitnum, type, target, mode);
 	}
     }
-- 
2.17.1

[PATCHv2 2/2] Improve do_store_flag for comparing single bit against that bit

[Andrew Pinski]

This is a case which I noticed while working on the previous patch.
Sometimes we end up with `a == CST` instead of comparing against 0.
This happens in the following code:
```
unsigned f(unsigned t)
{
  if (t & ~(1<<30)) __builtin_unreachable();
  t ^= (1<<30);
  return t != 0;
}
```

We should handle the case where the nonzero bits is the same as the
comparison operand.

Changes from v1:
* v2: Updated for the bit extraction changes.

OK? Bootstrapped and tested on x86_64-linux-gnu.

gcc/ChangeLog:

	* expr.cc (do_store_flag): Improve for single bit testing
	not against zero but against that single bit.
---
 gcc/expr.cc | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index b85e963b57e..e7e6112ff81 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -13152,12 +13152,15 @@ do_store_flag (sepops ops, rtx target, machine_mode mode)
      than an scc insn even if we have it.  */
 
   if ((code == NE || code == EQ)
-      && integer_zerop (arg1)
+      && (integer_zerop (arg1)
+	  || integer_pow2p (arg1))
       && (TYPE_PRECISION (ops->type) != 1 || TYPE_UNSIGNED (ops->type)))
     {
       wide_int nz = tree_nonzero_bits (arg0);
 
-      if (wi::popcount (nz) == 1)
+      if (wi::popcount (nz) == 1
+	  && (integer_zerop (arg1)
+	      || wi::to_wide (arg1) == nz))
 	{
 	  tree op0;
 	  int bitnum;
@@ -13175,7 +13178,9 @@ do_store_flag (sepops ops, rtx target, machine_mode mode)
 	      op0 = arg0;
 	      bitnum = wi::exact_log2 (nz);
 	    }
-	  enum tree_code tcode = code == NE ? NE_EXPR : EQ_EXPR;
+	  enum tree_code tcode = EQ_EXPR;
+	  if ((code == NE) ^ !integer_zerop (arg1))
+	    tcode = NE_EXPR;
 
 	  type = lang_hooks.types.type_for_mode (mode, unsignedp);
 	  return expand_single_bit_test (loc, tcode,
-- 
2.17.1

Re: [PATCHv2 1/2] Improve do_store_flag for single bit comparison against 0

[Jeff Law]

On 5/20/23 09:04, Andrew Pinski via Gcc-patches wrote:
> While working something else, I noticed we could improve
> the following function code generation:
> ```
> unsigned f(unsigned t)
> {
>    if (t & ~(1<<30)) __builtin_unreachable();
>    return t != 0;
> }
> ```
> Right know we just emit a comparison against 0 instead
> of just a shift right by 30.
> There is code in do_store_flag which already optimizes
> `(t & 1<<30) != 0` to `(t >> 30) & 1` (using bit extraction if available).
> This patch extends it to handle the case where we know t has a nonzero
> of just one bit set.
> 
> Changes from v1:
> * v2: Updated for the bit extraction improvements.
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (do_store_flag): Extend the one bit checking case
> 	to handle the case where we don't have an and but rather still
> 	one bit is known to be non-zero.
OK
jeff

Re: [PATCHv2 2/2] Improve do_store_flag for comparing single bit against that bit

[Jeff Law]

On 5/20/23 09:04, Andrew Pinski via Gcc-patches wrote:
> This is a case which I noticed while working on the previous patch.
> Sometimes we end up with `a == CST` instead of comparing against 0.
> This happens in the following code:
> ```
> unsigned f(unsigned t)
> {
>    if (t & ~(1<<30)) __builtin_unreachable();
>    t ^= (1<<30);
>    return t != 0;
> }
> ```
> 
> We should handle the case where the nonzero bits is the same as the
> comparison operand.
> 
> Changes from v1:
> * v2: Updated for the bit extraction changes.
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (do_store_flag): Improve for single bit testing
> 	not against zero but against that single bit.
OK
jeff