public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/7] Improve do_store_flag
@ 2023-05-20  2:14 Andrew Pinski
  2023-05-20  2:14 ` [PATCH 1/7] Move fold_single_bit_test to expr.cc from fold-const.cc Andrew Pinski
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Andrew Pinski @ 2023-05-20  2:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

This patch set improves do_store_flag for the single bit case.
We go back to expanding the code directly rather than building some
trees. Plus instead of using shift+and we use directly bit_field
extraction; this improves code generation on avr.

Andrew Pinski (7):
  Move fold_single_bit_test to expr.cc from fold-const.cc
  Inline and simplify fold_single_bit_test_into_sign_test into
    fold_single_bit_test
  Use get_def_for_expr in fold_single_bit_test
  Simplify fold_single_bit_test slightly
  Simplify fold_single_bit_test with respect to code
  Use BIT_FIELD_REF inside fold_single_bit_test
  Expand directly for single bit test

 gcc/expr.cc       |  91 ++++++++++++++++++++++++++++++++-----
 gcc/fold-const.cc | 112 ----------------------------------------------
 gcc/fold-const.h  |   1 -
 3 files changed, 81 insertions(+), 123 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/7] Move fold_single_bit_test to expr.cc from fold-const.cc
  2023-05-20  2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski
@ 2023-05-20  2:14 ` Andrew Pinski
  2023-05-20  4:43   ` Jeff Law
  2023-05-20  2:14 ` [PATCH 2/7] Inline and simplify fold_single_bit_test_into_sign_test into fold_single_bit_test Andrew Pinski
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Andrew Pinski @ 2023-05-20  2:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

This is part 1 of N patch set that will change the expansion
of `(A & C) != 0` from using trees to directly expanding so later
on we can do some cost analysis.

Since the only user of fold_single_bit_test is now
expand, move it to there.

OK? Bootstrapped and tested on x86_64-linux.

gcc/ChangeLog:

	* fold-const.cc (fold_single_bit_test_into_sign_test): Move to
	expr.cc.
	(fold_single_bit_test): Likewise.
	* expr.cc (fold_single_bit_test_into_sign_test): Move from fold-const.cc
	(fold_single_bit_test): Likewise and make static.
	* fold-const.h (fold_single_bit_test): Remove declaration.
---
 gcc/expr.cc       | 113 ++++++++++++++++++++++++++++++++++++++++++++++
 gcc/fold-const.cc | 112 ---------------------------------------------
 gcc/fold-const.h  |   1 -
 3 files changed, 113 insertions(+), 113 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index 5ede094e705..f999f81af4a 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12898,6 +12898,119 @@ maybe_optimize_sub_cmp_0 (enum tree_code code, tree *arg0, tree *arg1)
   *arg1 = treeop1;
 }
 \f
+
+
+/* If CODE with arguments ARG0 and ARG1 represents a single bit
+   equality/inequality test, then return a simplified form of the test
+   using a sign testing.  Otherwise return NULL.  TYPE is the desired
+   result type.  */
+
+static tree
+fold_single_bit_test_into_sign_test (location_t loc,
+				     enum tree_code code, tree arg0, tree arg1,
+				     tree result_type)
+{
+  /* If this is testing a single bit, we can optimize the test.  */
+  if ((code == NE_EXPR || code == EQ_EXPR)
+      && TREE_CODE (arg0) == BIT_AND_EXPR && integer_zerop (arg1)
+      && integer_pow2p (TREE_OPERAND (arg0, 1)))
+    {
+      /* If we have (A & C) != 0 where C is the sign bit of A, convert
+	 this into A < 0.  Similarly for (A & C) == 0 into A >= 0.  */
+      tree arg00 = sign_bit_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg0, 1));
+
+      if (arg00 != NULL_TREE
+	  /* This is only a win if casting to a signed type is cheap,
+	     i.e. when arg00's type is not a partial mode.  */
+	  && type_has_mode_precision_p (TREE_TYPE (arg00)))
+	{
+	  tree stype = signed_type_for (TREE_TYPE (arg00));
+	  return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
+			      result_type,
+			      fold_convert_loc (loc, stype, arg00),
+			      build_int_cst (stype, 0));
+	}
+    }
+
+  return NULL_TREE;
+}
+
+/* If CODE with arguments ARG0 and ARG1 represents a single bit
+   equality/inequality test, then return a simplified form of
+   the test using shifts and logical operations.  Otherwise return
+   NULL.  TYPE is the desired result type.  */
+
+static tree
+fold_single_bit_test (location_t loc, enum tree_code code,
+		      tree arg0, tree arg1, tree result_type)
+{
+  /* If this is testing a single bit, we can optimize the test.  */
+  if ((code == NE_EXPR || code == EQ_EXPR)
+      && TREE_CODE (arg0) == BIT_AND_EXPR && integer_zerop (arg1)
+      && integer_pow2p (TREE_OPERAND (arg0, 1)))
+    {
+      tree inner = TREE_OPERAND (arg0, 0);
+      tree type = TREE_TYPE (arg0);
+      int bitnum = tree_log2 (TREE_OPERAND (arg0, 1));
+      scalar_int_mode operand_mode = SCALAR_INT_TYPE_MODE (type);
+      int ops_unsigned;
+      tree signed_type, unsigned_type, intermediate_type;
+      tree tem, one;
+
+      /* First, see if we can fold the single bit test into a sign-bit
+	 test.  */
+      tem = fold_single_bit_test_into_sign_test (loc, code, arg0, arg1,
+						 result_type);
+      if (tem)
+	return tem;
+
+      /* Otherwise we have (A & C) != 0 where C is a single bit,
+	 convert that into ((A >> C2) & 1).  Where C2 = log2(C).
+	 Similarly for (A & C) == 0.  */
+
+      /* If INNER is a right shift of a constant and it plus BITNUM does
+	 not overflow, adjust BITNUM and INNER.  */
+      if (TREE_CODE (inner) == RSHIFT_EXPR
+	  && TREE_CODE (TREE_OPERAND (inner, 1)) == INTEGER_CST
+	  && bitnum < TYPE_PRECISION (type)
+	  && wi::ltu_p (wi::to_wide (TREE_OPERAND (inner, 1)),
+			TYPE_PRECISION (type) - bitnum))
+	{
+	  bitnum += tree_to_uhwi (TREE_OPERAND (inner, 1));
+	  inner = TREE_OPERAND (inner, 0);
+	}
+
+      /* If we are going to be able to omit the AND below, we must do our
+	 operations as unsigned.  If we must use the AND, we have a choice.
+	 Normally unsigned is faster, but for some machines signed is.  */
+      ops_unsigned = (load_extend_op (operand_mode) == SIGN_EXTEND
+		      && !flag_syntax_only) ? 0 : 1;
+
+      signed_type = lang_hooks.types.type_for_mode (operand_mode, 0);
+      unsigned_type = lang_hooks.types.type_for_mode (operand_mode, 1);
+      intermediate_type = ops_unsigned ? unsigned_type : signed_type;
+      inner = fold_convert_loc (loc, intermediate_type, inner);
+
+      if (bitnum != 0)
+	inner = build2 (RSHIFT_EXPR, intermediate_type,
+			inner, size_int (bitnum));
+
+      one = build_int_cst (intermediate_type, 1);
+
+      if (code == EQ_EXPR)
+	inner = fold_build2_loc (loc, BIT_XOR_EXPR, intermediate_type, inner, one);
+
+      /* Put the AND last so it can combine with more things.  */
+      inner = build2 (BIT_AND_EXPR, intermediate_type, inner, one);
+
+      /* Make sure to return the proper type.  */
+      inner = fold_convert_loc (loc, result_type, inner);
+
+      return inner;
+    }
+  return NULL_TREE;
+}
+
 /* Generate code to calculate OPS, and exploded expression
    using a store-flag instruction and return an rtx for the result.
    OPS reflects a comparison.
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index a73b972ab9a..25466e97220 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -7433,118 +7433,6 @@ fold_div_compare (enum tree_code code, tree c1, tree c2, tree *lo,
   return code;
 }
 
-
-/* If CODE with arguments ARG0 and ARG1 represents a single bit
-   equality/inequality test, then return a simplified form of the test
-   using a sign testing.  Otherwise return NULL.  TYPE is the desired
-   result type.  */
-
-static tree
-fold_single_bit_test_into_sign_test (location_t loc,
-				     enum tree_code code, tree arg0, tree arg1,
-				     tree result_type)
-{
-  /* If this is testing a single bit, we can optimize the test.  */
-  if ((code == NE_EXPR || code == EQ_EXPR)
-      && TREE_CODE (arg0) == BIT_AND_EXPR && integer_zerop (arg1)
-      && integer_pow2p (TREE_OPERAND (arg0, 1)))
-    {
-      /* If we have (A & C) != 0 where C is the sign bit of A, convert
-	 this into A < 0.  Similarly for (A & C) == 0 into A >= 0.  */
-      tree arg00 = sign_bit_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg0, 1));
-
-      if (arg00 != NULL_TREE
-	  /* This is only a win if casting to a signed type is cheap,
-	     i.e. when arg00's type is not a partial mode.  */
-	  && type_has_mode_precision_p (TREE_TYPE (arg00)))
-	{
-	  tree stype = signed_type_for (TREE_TYPE (arg00));
-	  return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
-			      result_type,
-			      fold_convert_loc (loc, stype, arg00),
-			      build_int_cst (stype, 0));
-	}
-    }
-
-  return NULL_TREE;
-}
-
-/* If CODE with arguments ARG0 and ARG1 represents a single bit
-   equality/inequality test, then return a simplified form of
-   the test using shifts and logical operations.  Otherwise return
-   NULL.  TYPE is the desired result type.  */
-
-tree
-fold_single_bit_test (location_t loc, enum tree_code code,
-		      tree arg0, tree arg1, tree result_type)
-{
-  /* If this is testing a single bit, we can optimize the test.  */
-  if ((code == NE_EXPR || code == EQ_EXPR)
-      && TREE_CODE (arg0) == BIT_AND_EXPR && integer_zerop (arg1)
-      && integer_pow2p (TREE_OPERAND (arg0, 1)))
-    {
-      tree inner = TREE_OPERAND (arg0, 0);
-      tree type = TREE_TYPE (arg0);
-      int bitnum = tree_log2 (TREE_OPERAND (arg0, 1));
-      scalar_int_mode operand_mode = SCALAR_INT_TYPE_MODE (type);
-      int ops_unsigned;
-      tree signed_type, unsigned_type, intermediate_type;
-      tree tem, one;
-
-      /* First, see if we can fold the single bit test into a sign-bit
-	 test.  */
-      tem = fold_single_bit_test_into_sign_test (loc, code, arg0, arg1,
-						 result_type);
-      if (tem)
-	return tem;
-
-      /* Otherwise we have (A & C) != 0 where C is a single bit,
-	 convert that into ((A >> C2) & 1).  Where C2 = log2(C).
-	 Similarly for (A & C) == 0.  */
-
-      /* If INNER is a right shift of a constant and it plus BITNUM does
-	 not overflow, adjust BITNUM and INNER.  */
-      if (TREE_CODE (inner) == RSHIFT_EXPR
-	  && TREE_CODE (TREE_OPERAND (inner, 1)) == INTEGER_CST
-	  && bitnum < TYPE_PRECISION (type)
-	  && wi::ltu_p (wi::to_wide (TREE_OPERAND (inner, 1)),
-			TYPE_PRECISION (type) - bitnum))
-	{
-	  bitnum += tree_to_uhwi (TREE_OPERAND (inner, 1));
-	  inner = TREE_OPERAND (inner, 0);
-	}
-
-      /* If we are going to be able to omit the AND below, we must do our
-	 operations as unsigned.  If we must use the AND, we have a choice.
-	 Normally unsigned is faster, but for some machines signed is.  */
-      ops_unsigned = (load_extend_op (operand_mode) == SIGN_EXTEND
-		      && !flag_syntax_only) ? 0 : 1;
-
-      signed_type = lang_hooks.types.type_for_mode (operand_mode, 0);
-      unsigned_type = lang_hooks.types.type_for_mode (operand_mode, 1);
-      intermediate_type = ops_unsigned ? unsigned_type : signed_type;
-      inner = fold_convert_loc (loc, intermediate_type, inner);
-
-      if (bitnum != 0)
-	inner = build2 (RSHIFT_EXPR, intermediate_type,
-			inner, size_int (bitnum));
-
-      one = build_int_cst (intermediate_type, 1);
-
-      if (code == EQ_EXPR)
-	inner = fold_build2_loc (loc, BIT_XOR_EXPR, intermediate_type, inner, one);
-
-      /* Put the AND last so it can combine with more things.  */
-      inner = build2 (BIT_AND_EXPR, intermediate_type, inner, one);
-
-      /* Make sure to return the proper type.  */
-      inner = fold_convert_loc (loc, result_type, inner);
-
-      return inner;
-    }
-  return NULL_TREE;
-}
-
 /* Test whether it is preferable to swap two operands, ARG0 and
    ARG1, for example because ARG0 is an integer constant and ARG1
    isn't.  */
diff --git a/gcc/fold-const.h b/gcc/fold-const.h
index b828badc42f..24c50fcc557 100644
--- a/gcc/fold-const.h
+++ b/gcc/fold-const.h
@@ -92,7 +92,6 @@ extern bool fold_convertible_p (const_tree, const_tree);
 #define fold_convert(T1,T2)\
    fold_convert_loc (UNKNOWN_LOCATION, T1, T2)
 extern tree fold_convert_loc (location_t, tree, tree);
-extern tree fold_single_bit_test (location_t, enum tree_code, tree, tree, tree);
 extern tree fold_ignored_result (tree);
 extern tree fold_abs_const (tree, tree);
 extern tree fold_indirect_ref_1 (location_t, tree, tree);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/7] Inline and simplify fold_single_bit_test_into_sign_test into fold_single_bit_test
  2023-05-20  2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski
  2023-05-20  2:14 ` [PATCH 1/7] Move fold_single_bit_test to expr.cc from fold-const.cc Andrew Pinski
@ 2023-05-20  2:14 ` Andrew Pinski
  2023-05-20  4:47   ` Jeff Law
  2023-05-20  4:48   ` Jeff Law
  2023-05-20  2:14 ` [PATCH 3/7] Use get_def_for_expr in fold_single_bit_test Andrew Pinski
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Andrew Pinski @ 2023-05-20  2:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

Since the last use of fold_single_bit_test is fold_single_bit_test,
we can inline it and even simplify the inlined version. This has
no behavior change.

OK? Bootstrapped and tested on x86_64-linux.

gcc/ChangeLog:

	* expr.cc (fold_single_bit_test_into_sign_test): Inline into ...
	(fold_single_bit_test): This and simplify.
---
 gcc/expr.cc | 51 ++++++++++-----------------------------------------
 1 file changed, 10 insertions(+), 41 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index f999f81af4a..6221b6991c5 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12899,42 +12899,6 @@ maybe_optimize_sub_cmp_0 (enum tree_code code, tree *arg0, tree *arg1)
 }
 \f
 
-
-/* If CODE with arguments ARG0 and ARG1 represents a single bit
-   equality/inequality test, then return a simplified form of the test
-   using a sign testing.  Otherwise return NULL.  TYPE is the desired
-   result type.  */
-
-static tree
-fold_single_bit_test_into_sign_test (location_t loc,
-				     enum tree_code code, tree arg0, tree arg1,
-				     tree result_type)
-{
-  /* If this is testing a single bit, we can optimize the test.  */
-  if ((code == NE_EXPR || code == EQ_EXPR)
-      && TREE_CODE (arg0) == BIT_AND_EXPR && integer_zerop (arg1)
-      && integer_pow2p (TREE_OPERAND (arg0, 1)))
-    {
-      /* If we have (A & C) != 0 where C is the sign bit of A, convert
-	 this into A < 0.  Similarly for (A & C) == 0 into A >= 0.  */
-      tree arg00 = sign_bit_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg0, 1));
-
-      if (arg00 != NULL_TREE
-	  /* This is only a win if casting to a signed type is cheap,
-	     i.e. when arg00's type is not a partial mode.  */
-	  && type_has_mode_precision_p (TREE_TYPE (arg00)))
-	{
-	  tree stype = signed_type_for (TREE_TYPE (arg00));
-	  return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
-			      result_type,
-			      fold_convert_loc (loc, stype, arg00),
-			      build_int_cst (stype, 0));
-	}
-    }
-
-  return NULL_TREE;
-}
-
 /* If CODE with arguments ARG0 and ARG1 represents a single bit
    equality/inequality test, then return a simplified form of
    the test using shifts and logical operations.  Otherwise return
@@ -12955,14 +12919,19 @@ fold_single_bit_test (location_t loc, enum tree_code code,
       scalar_int_mode operand_mode = SCALAR_INT_TYPE_MODE (type);
       int ops_unsigned;
       tree signed_type, unsigned_type, intermediate_type;
-      tree tem, one;
+      tree one;
 
       /* First, see if we can fold the single bit test into a sign-bit
 	 test.  */
-      tem = fold_single_bit_test_into_sign_test (loc, code, arg0, arg1,
-						 result_type);
-      if (tem)
-	return tem;
+      if (bitnum == TYPE_PRECISION (type) - 1
+	  && type_has_mode_precision_p (type))
+	{
+	  tree stype = signed_type_for (type);
+	  return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
+				  result_type,
+				  fold_convert_loc (loc, stype, inner),
+				  build_int_cst (stype, 0));
+	}
 
       /* Otherwise we have (A & C) != 0 where C is a single bit,
 	 convert that into ((A >> C2) & 1).  Where C2 = log2(C).
-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/7] Use get_def_for_expr in fold_single_bit_test
  2023-05-20  2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski
  2023-05-20  2:14 ` [PATCH 1/7] Move fold_single_bit_test to expr.cc from fold-const.cc Andrew Pinski
  2023-05-20  2:14 ` [PATCH 2/7] Inline and simplify fold_single_bit_test_into_sign_test into fold_single_bit_test Andrew Pinski
@ 2023-05-20  2:14 ` Andrew Pinski
  2023-05-20  4:49   ` Jeff Law
  2023-05-20  2:14 ` [PATCH 4/7] Simplify fold_single_bit_test slightly Andrew Pinski
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Andrew Pinski @ 2023-05-20  2:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

The code in fold_single_bit_test, checks if
the inner was a right shift and improve the bitnum
based on that. But since the inner will always be a
SSA_NAME at this point, the code is dead. Move it over
to use the helper function get_def_for_expr instead.

OK? Bootstrapped and tested on x86_64-linux.

gcc/ChangeLog:

	* expr.cc (fold_single_bit_test): Use get_def_for_expr
	instead of checking the inner's code.
---
 gcc/expr.cc | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index 6221b6991c5..a61772b6808 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12920,6 +12920,7 @@ fold_single_bit_test (location_t loc, enum tree_code code,
       int ops_unsigned;
       tree signed_type, unsigned_type, intermediate_type;
       tree one;
+      gimple *inner_def;
 
       /* First, see if we can fold the single bit test into a sign-bit
 	 test.  */
@@ -12939,14 +12940,14 @@ fold_single_bit_test (location_t loc, enum tree_code code,
 
       /* If INNER is a right shift of a constant and it plus BITNUM does
 	 not overflow, adjust BITNUM and INNER.  */
-      if (TREE_CODE (inner) == RSHIFT_EXPR
-	  && TREE_CODE (TREE_OPERAND (inner, 1)) == INTEGER_CST
+      if ((inner_def = get_def_for_expr (inner, RSHIFT_EXPR))
+	  && TREE_CODE (gimple_assign_rhs2 (inner_def)) == INTEGER_CST
 	  && bitnum < TYPE_PRECISION (type)
-	  && wi::ltu_p (wi::to_wide (TREE_OPERAND (inner, 1)),
+	  && wi::ltu_p (wi::to_wide (gimple_assign_rhs2 (inner_def)),
 			TYPE_PRECISION (type) - bitnum))
 	{
-	  bitnum += tree_to_uhwi (TREE_OPERAND (inner, 1));
-	  inner = TREE_OPERAND (inner, 0);
+	  bitnum += tree_to_uhwi (gimple_assign_rhs2 (inner_def));
+	  inner = gimple_assign_rhs1 (inner_def);
 	}
 
       /* If we are going to be able to omit the AND below, we must do our
-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/7] Simplify fold_single_bit_test slightly
  2023-05-20  2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski
                   ` (2 preceding siblings ...)
  2023-05-20  2:14 ` [PATCH 3/7] Use get_def_for_expr in fold_single_bit_test Andrew Pinski
@ 2023-05-20  2:14 ` Andrew Pinski
  2023-05-20  4:51   ` Jeff Law
  2023-05-20  2:14 ` [PATCH 5/7] Simplify fold_single_bit_test with respect to code Andrew Pinski
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Andrew Pinski @ 2023-05-20  2:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

Now the only use of fold_single_bit_test is in do_store_flag,
we can change it such that to pass the inner arg and bitnum
instead of building a tree. There is no code generation changes
due to this change, only a decrease in GC memory that is produced
during expansion.

OK? Bootstrapped and tested on x86_64-linux.

gcc/ChangeLog:

	* expr.cc (fold_single_bit_test): Take inner and bitnum
	instead of arg0 and arg1. Update the code.
	(do_store_flag): Don't create a tree when calling
	fold_single_bit_test instead just call it with the bitnum
	and the inner tree.
---
 gcc/expr.cc | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index a61772b6808..67a9f82ca17 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12899,23 +12899,19 @@ maybe_optimize_sub_cmp_0 (enum tree_code code, tree *arg0, tree *arg1)
 }
 \f
 
-/* If CODE with arguments ARG0 and ARG1 represents a single bit
+/* If CODE with arguments INNER & (1<<BITNUM) and 0 represents a single bit
    equality/inequality test, then return a simplified form of
    the test using shifts and logical operations.  Otherwise return
    NULL.  TYPE is the desired result type.  */
 
 static tree
 fold_single_bit_test (location_t loc, enum tree_code code,
-		      tree arg0, tree arg1, tree result_type)
+		      tree inner, int bitnum,
+		      tree result_type)
 {
-  /* If this is testing a single bit, we can optimize the test.  */
-  if ((code == NE_EXPR || code == EQ_EXPR)
-      && TREE_CODE (arg0) == BIT_AND_EXPR && integer_zerop (arg1)
-      && integer_pow2p (TREE_OPERAND (arg0, 1)))
+  if ((code == NE_EXPR || code == EQ_EXPR))
     {
-      tree inner = TREE_OPERAND (arg0, 0);
-      tree type = TREE_TYPE (arg0);
-      int bitnum = tree_log2 (TREE_OPERAND (arg0, 1));
+      tree type = TREE_TYPE (inner);
       scalar_int_mode operand_mode = SCALAR_INT_TYPE_MODE (type);
       int ops_unsigned;
       tree signed_type, unsigned_type, intermediate_type;
@@ -13170,12 +13166,14 @@ do_store_flag (sepops ops, rtx target, machine_mode mode)
       if (srcstmt
 	  && integer_pow2p (gimple_assign_rhs2 (srcstmt)))
 	{
+	  tree temp;
 	  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);
-	  tree temp = fold_build2_loc (loc, BIT_AND_EXPR, TREE_TYPE (arg1),
+	  temp = fold_single_bit_test (loc, tcode,
 				       gimple_assign_rhs1 (srcstmt),
-				       gimple_assign_rhs2 (srcstmt));
-	  temp = fold_single_bit_test (loc, tcode, temp, arg1, type);
+				       bitnum, type);
 	  if (temp)
 	    return expand_expr (temp, target, VOIDmode, EXPAND_NORMAL);
 	}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/7] Simplify fold_single_bit_test with respect to code
  2023-05-20  2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski
                   ` (3 preceding siblings ...)
  2023-05-20  2:14 ` [PATCH 4/7] Simplify fold_single_bit_test slightly Andrew Pinski
@ 2023-05-20  2:14 ` Andrew Pinski
  2023-05-20  4:52   ` Jeff Law
  2023-05-20  2:14 ` [PATCH 6/7] Use BIT_FIELD_REF inside fold_single_bit_test Andrew Pinski
  2023-05-20  2:14 ` [PATCH 7/7] Expand directly for single bit test Andrew Pinski
  6 siblings, 1 reply; 21+ messages in thread
From: Andrew Pinski @ 2023-05-20  2:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

Since we know that fold_single_bit_test is now only passed
NE_EXPR or EQ_EXPR, we can simplify it and just use a gcc_assert
to assert that is the code that is being passed.

OK? Bootstrapped and tested on x86_64-linux.

gcc/ChangeLog:

	* expr.cc (fold_single_bit_test): Add an assert
	and simplify based on code being NE_EXPR or EQ_EXPR.
---
 gcc/expr.cc | 108 ++++++++++++++++++++++++++--------------------------
 1 file changed, 53 insertions(+), 55 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index 67a9f82ca17..b5bc3fabb7e 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12909,72 +12909,70 @@ fold_single_bit_test (location_t loc, enum tree_code code,
 		      tree inner, int bitnum,
 		      tree result_type)
 {
-  if ((code == NE_EXPR || code == EQ_EXPR))
-    {
-      tree type = TREE_TYPE (inner);
-      scalar_int_mode operand_mode = SCALAR_INT_TYPE_MODE (type);
-      int ops_unsigned;
-      tree signed_type, unsigned_type, intermediate_type;
-      tree one;
-      gimple *inner_def;
+  gcc_assert (code == NE_EXPR || code == EQ_EXPR);
 
-      /* First, see if we can fold the single bit test into a sign-bit
-	 test.  */
-      if (bitnum == TYPE_PRECISION (type) - 1
-	  && type_has_mode_precision_p (type))
-	{
-	  tree stype = signed_type_for (type);
-	  return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
-				  result_type,
-				  fold_convert_loc (loc, stype, inner),
-				  build_int_cst (stype, 0));
-	}
+  tree type = TREE_TYPE (inner);
+  scalar_int_mode operand_mode = SCALAR_INT_TYPE_MODE (type);
+  int ops_unsigned;
+  tree signed_type, unsigned_type, intermediate_type;
+  tree one;
+  gimple *inner_def;
 
-      /* Otherwise we have (A & C) != 0 where C is a single bit,
-	 convert that into ((A >> C2) & 1).  Where C2 = log2(C).
-	 Similarly for (A & C) == 0.  */
+  /* First, see if we can fold the single bit test into a sign-bit
+     test.  */
+  if (bitnum == TYPE_PRECISION (type) - 1
+      && type_has_mode_precision_p (type))
+    {
+      tree stype = signed_type_for (type);
+      return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
+			      result_type,
+			      fold_convert_loc (loc, stype, inner),
+			      build_int_cst (stype, 0));
+    }
 
-      /* If INNER is a right shift of a constant and it plus BITNUM does
-	 not overflow, adjust BITNUM and INNER.  */
-      if ((inner_def = get_def_for_expr (inner, RSHIFT_EXPR))
-	  && TREE_CODE (gimple_assign_rhs2 (inner_def)) == INTEGER_CST
-	  && bitnum < TYPE_PRECISION (type)
-	  && wi::ltu_p (wi::to_wide (gimple_assign_rhs2 (inner_def)),
-			TYPE_PRECISION (type) - bitnum))
-	{
-	  bitnum += tree_to_uhwi (gimple_assign_rhs2 (inner_def));
-	  inner = gimple_assign_rhs1 (inner_def);
-	}
+  /* Otherwise we have (A & C) != 0 where C is a single bit,
+     convert that into ((A >> C2) & 1).  Where C2 = log2(C).
+     Similarly for (A & C) == 0.  */
 
-      /* If we are going to be able to omit the AND below, we must do our
-	 operations as unsigned.  If we must use the AND, we have a choice.
-	 Normally unsigned is faster, but for some machines signed is.  */
-      ops_unsigned = (load_extend_op (operand_mode) == SIGN_EXTEND
-		      && !flag_syntax_only) ? 0 : 1;
+  /* If INNER is a right shift of a constant and it plus BITNUM does
+     not overflow, adjust BITNUM and INNER.  */
+  if ((inner_def = get_def_for_expr (inner, RSHIFT_EXPR))
+       && TREE_CODE (gimple_assign_rhs2 (inner_def)) == INTEGER_CST
+       && bitnum < TYPE_PRECISION (type)
+       && wi::ltu_p (wi::to_wide (gimple_assign_rhs2 (inner_def)),
+		     TYPE_PRECISION (type) - bitnum))
+    {
+      bitnum += tree_to_uhwi (gimple_assign_rhs2 (inner_def));
+      inner = gimple_assign_rhs1 (inner_def);
+    }
 
-      signed_type = lang_hooks.types.type_for_mode (operand_mode, 0);
-      unsigned_type = lang_hooks.types.type_for_mode (operand_mode, 1);
-      intermediate_type = ops_unsigned ? unsigned_type : signed_type;
-      inner = fold_convert_loc (loc, intermediate_type, inner);
+  /* If we are going to be able to omit the AND below, we must do our
+     operations as unsigned.  If we must use the AND, we have a choice.
+     Normally unsigned is faster, but for some machines signed is.  */
+  ops_unsigned = (load_extend_op (operand_mode) == SIGN_EXTEND
+		  && !flag_syntax_only) ? 0 : 1;
 
-      if (bitnum != 0)
-	inner = build2 (RSHIFT_EXPR, intermediate_type,
-			inner, size_int (bitnum));
+  signed_type = lang_hooks.types.type_for_mode (operand_mode, 0);
+  unsigned_type = lang_hooks.types.type_for_mode (operand_mode, 1);
+  intermediate_type = ops_unsigned ? unsigned_type : signed_type;
+  inner = fold_convert_loc (loc, intermediate_type, inner);
 
-      one = build_int_cst (intermediate_type, 1);
+  if (bitnum != 0)
+    inner = build2 (RSHIFT_EXPR, intermediate_type,
+		    inner, size_int (bitnum));
 
-      if (code == EQ_EXPR)
-	inner = fold_build2_loc (loc, BIT_XOR_EXPR, intermediate_type, inner, one);
+  one = build_int_cst (intermediate_type, 1);
 
-      /* Put the AND last so it can combine with more things.  */
-      inner = build2 (BIT_AND_EXPR, intermediate_type, inner, one);
+  if (code == EQ_EXPR)
+    inner = fold_build2_loc (loc, BIT_XOR_EXPR, intermediate_type, inner, one);
 
-      /* Make sure to return the proper type.  */
-      inner = fold_convert_loc (loc, result_type, inner);
+  /* Put the AND last so it can combine with more things.  */
+  inner = build2 (BIT_AND_EXPR, intermediate_type, inner, one);
 
-      return inner;
-    }
-  return NULL_TREE;
+  /* Make sure to return the proper type.  */
+  inner = fold_convert_loc (loc, result_type, inner);
+
+  return inner;
 }
 
 /* Generate code to calculate OPS, and exploded expression
-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 6/7] Use BIT_FIELD_REF inside fold_single_bit_test
  2023-05-20  2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski
                   ` (4 preceding siblings ...)
  2023-05-20  2:14 ` [PATCH 5/7] Simplify fold_single_bit_test with respect to code Andrew Pinski
@ 2023-05-20  2:14 ` Andrew Pinski
  2023-05-20  4:54   ` Jeff Law
  2023-05-20  2:14 ` [PATCH 7/7] Expand directly for single bit test Andrew Pinski
  6 siblings, 1 reply; 21+ messages in thread
From: Andrew Pinski @ 2023-05-20  2:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

Instead of depending on combine to do the extraction,
Let's create a tree which will expand directly into
the extraction. This improves code generation on some
targets.

OK? Bootstrapped and tested on x86_64-linux.

gcc/ChangeLog:

	* expr.cc (fold_single_bit_test): Use BIT_FIELD_REF
	instead of shift/and.
---
 gcc/expr.cc | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index b5bc3fabb7e..d04e8ed0204 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12957,22 +12957,21 @@ fold_single_bit_test (location_t loc, enum tree_code code,
   intermediate_type = ops_unsigned ? unsigned_type : signed_type;
   inner = fold_convert_loc (loc, intermediate_type, inner);
 
-  if (bitnum != 0)
-    inner = build2 (RSHIFT_EXPR, intermediate_type,
-		    inner, size_int (bitnum));
+  tree bftype = build_nonstandard_integer_type (1, 1);
+  int bitpos = bitnum;
 
-  one = build_int_cst (intermediate_type, 1);
+  if (BYTES_BIG_ENDIAN)
+    bitpos = GET_MODE_BITSIZE (operand_mode) - 1 - bitpos;
 
-  if (code == EQ_EXPR)
-    inner = fold_build2_loc (loc, BIT_XOR_EXPR, intermediate_type, inner, one);
+  inner = build3_loc (loc, BIT_FIELD_REF, bftype, inner,
+		      bitsize_int (1), bitsize_int (bitpos));
 
-  /* Put the AND last so it can combine with more things.  */
-  inner = build2 (BIT_AND_EXPR, intermediate_type, inner, one);
+  one = build_int_cst (bftype, 1);
 
-  /* Make sure to return the proper type.  */
-  inner = fold_convert_loc (loc, result_type, inner);
+  if (code == EQ_EXPR)
+    inner = fold_build2_loc (loc, BIT_XOR_EXPR, bftype, inner, one);
 
-  return inner;
+  return fold_convert_loc (loc, result_type, inner);
 }
 
 /* Generate code to calculate OPS, and exploded expression
-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 7/7] Expand directly for single bit test
  2023-05-20  2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski
                   ` (5 preceding siblings ...)
  2023-05-20  2:14 ` [PATCH 6/7] Use BIT_FIELD_REF inside fold_single_bit_test Andrew Pinski
@ 2023-05-20  2:14 ` Andrew Pinski
  2023-05-20  4:55   ` Jeff Law
  6 siblings, 1 reply; 21+ messages in thread
From: Andrew Pinski @ 2023-05-20  2:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

Instead of using creating trees to the expansion,
just expand directly which makes the code a little simplier
but also reduces how much GC memory will be used during the expansion.

OK? Bootstrapped and tested on x86_64-linux.

gcc/ChangeLog:

	* expr.cc (fold_single_bit_test): Rename to ...
	(expand_single_bit_test): This and expand directly.
	(do_store_flag): Update for the rename function.
---
 gcc/expr.cc | 63 ++++++++++++++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 35 deletions(-)

diff --git a/gcc/expr.cc b/gcc/expr.cc
index d04e8ed0204..6849c9627d0 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12899,15 +12899,14 @@ maybe_optimize_sub_cmp_0 (enum tree_code code, tree *arg0, tree *arg1)
 }
 \f
 
-/* If CODE with arguments INNER & (1<<BITNUM) and 0 represents a single bit
-   equality/inequality test, then return a simplified form of
-   the test using shifts and logical operations.  Otherwise return
-   NULL.  TYPE is the desired result type.  */
+/* Expand CODE with arguments INNER & (1<<BITNUM) and 0 that represents
+   a single bit equality/inequality test, returns where the result is located.  */
 
-static tree
-fold_single_bit_test (location_t loc, enum tree_code code,
-		      tree inner, int bitnum,
-		      tree result_type)
+static rtx
+expand_single_bit_test (location_t loc, enum tree_code code,
+			tree inner, int bitnum,
+			tree result_type, rtx target,
+			machine_mode mode)
 {
   gcc_assert (code == NE_EXPR || code == EQ_EXPR);
 
@@ -12915,7 +12914,6 @@ fold_single_bit_test (location_t loc, enum tree_code code,
   scalar_int_mode operand_mode = SCALAR_INT_TYPE_MODE (type);
   int ops_unsigned;
   tree signed_type, unsigned_type, intermediate_type;
-  tree one;
   gimple *inner_def;
 
   /* First, see if we can fold the single bit test into a sign-bit
@@ -12924,10 +12922,11 @@ fold_single_bit_test (location_t loc, enum tree_code code,
       && type_has_mode_precision_p (type))
     {
       tree stype = signed_type_for (type);
-      return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
-			      result_type,
-			      fold_convert_loc (loc, stype, inner),
-			      build_int_cst (stype, 0));
+      tree tmp = fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
+				  result_type,
+				  fold_convert_loc (loc, stype, inner),
+				  build_int_cst (stype, 0));
+      return expand_expr (tmp, target, VOIDmode, EXPAND_NORMAL);
     }
 
   /* Otherwise we have (A & C) != 0 where C is a single bit,
@@ -12957,21 +12956,21 @@ fold_single_bit_test (location_t loc, enum tree_code code,
   intermediate_type = ops_unsigned ? unsigned_type : signed_type;
   inner = fold_convert_loc (loc, intermediate_type, inner);
 
-  tree bftype = build_nonstandard_integer_type (1, 1);
-  int bitpos = bitnum;
-
-  if (BYTES_BIG_ENDIAN)
-    bitpos = GET_MODE_BITSIZE (operand_mode) - 1 - bitpos;
+  rtx inner0 = expand_expr (inner, target, VOIDmode, EXPAND_NORMAL);
 
-  inner = build3_loc (loc, BIT_FIELD_REF, bftype, inner,
-		      bitsize_int (1), bitsize_int (bitpos));
-
-  one = build_int_cst (bftype, 1);
+  inner0 = extract_bit_field (inner0, 1, bitnum, 1, target,
+			      operand_mode, mode, 0, NULL);
 
   if (code == EQ_EXPR)
-    inner = fold_build2_loc (loc, BIT_XOR_EXPR, bftype, inner, one);
-
-  return fold_convert_loc (loc, result_type, inner);
+    inner0 = expand_binop (GET_MODE (inner0), xor_optab, inner0, const1_rtx,
+			   NULL_RTX, 1, OPTAB_LIB_WIDEN);
+  if (GET_MODE (inner0) != mode)
+    {
+      rtx t = gen_reg_rtx (mode);
+      convert_move (t, inner0, 0);
+      return t;
+    }
+  return inner0;
 }
 
 /* Generate code to calculate OPS, and exploded expression
@@ -13150,10 +13149,7 @@ do_store_flag (sepops ops, rtx target, machine_mode mode)
      do this by shifting the bit being tested to the low-order bit and
      masking the result with the constant 1.  If the condition was EQ,
      we xor it with 1.  This does not require an scc insn and is faster
-     than an scc insn even if we have it.
-
-     The code to make this transformation was moved into fold_single_bit_test,
-     so we just call into the folder and expand its result.  */
+     than an scc insn even if we have it.  */
 
   if ((code == NE || code == EQ)
       && integer_zerop (arg1)
@@ -13163,16 +13159,13 @@ do_store_flag (sepops ops, rtx target, machine_mode mode)
       if (srcstmt
 	  && integer_pow2p (gimple_assign_rhs2 (srcstmt)))
 	{
-	  tree temp;
 	  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);
-	  temp = fold_single_bit_test (loc, tcode,
-				       gimple_assign_rhs1 (srcstmt),
-				       bitnum, type);
-	  if (temp)
-	    return expand_expr (temp, target, VOIDmode, EXPAND_NORMAL);
+	  return expand_single_bit_test (loc, tcode,
+					 gimple_assign_rhs1 (srcstmt),
+					 bitnum, type, target, mode);
 	}
     }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] Move fold_single_bit_test to expr.cc from fold-const.cc
  2023-05-20  2:14 ` [PATCH 1/7] Move fold_single_bit_test to expr.cc from fold-const.cc Andrew Pinski
@ 2023-05-20  4:43   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-20  4:43 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/19/23 20:14, Andrew Pinski via Gcc-patches wrote:
> This is part 1 of N patch set that will change the expansion
> of `(A & C) != 0` from using trees to directly expanding so later
> on we can do some cost analysis.
> 
> Since the only user of fold_single_bit_test is now
> expand, move it to there.
> 
> OK? Bootstrapped and tested on x86_64-linux.
> 
> gcc/ChangeLog:
> 
> 	* fold-const.cc (fold_single_bit_test_into_sign_test): Move to
> 	expr.cc.
> 	(fold_single_bit_test): Likewise.
> 	* expr.cc (fold_single_bit_test_into_sign_test): Move from fold-const.cc
> 	(fold_single_bit_test): Likewise and make static.
> 	* fold-const.h (fold_single_bit_test): Remove declaration.
I'm assuming this is purely moving the bits around.

OK.

jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] Inline and simplify fold_single_bit_test_into_sign_test into fold_single_bit_test
  2023-05-20  2:14 ` [PATCH 2/7] Inline and simplify fold_single_bit_test_into_sign_test into fold_single_bit_test Andrew Pinski
@ 2023-05-20  4:47   ` Jeff Law
  2023-05-20  4:48   ` Jeff Law
  1 sibling, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-20  4:47 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/19/23 20:14, Andrew Pinski via Gcc-patches wrote:
> Since the last use of fold_single_bit_test is fold_single_bit_test,
> we can inline it and even simplify the inlined version. This has
> no behavior change.
> 
> OK? Bootstrapped and tested on x86_64-linux.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (fold_single_bit_test_into_sign_test): Inline into ...
> 	(fold_single_bit_test): This and simplify.
Going to trust the inlining and simpification is really NFC.  It's not 
really obvious from the patch.

jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] Inline and simplify fold_single_bit_test_into_sign_test into fold_single_bit_test
  2023-05-20  2:14 ` [PATCH 2/7] Inline and simplify fold_single_bit_test_into_sign_test into fold_single_bit_test Andrew Pinski
  2023-05-20  4:47   ` Jeff Law
@ 2023-05-20  4:48   ` Jeff Law
  1 sibling, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-20  4:48 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/19/23 20:14, Andrew Pinski via Gcc-patches wrote:
> Since the last use of fold_single_bit_test is fold_single_bit_test,
> we can inline it and even simplify the inlined version. This has
> no behavior change.
> 
> OK? Bootstrapped and tested on x86_64-linux.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (fold_single_bit_test_into_sign_test): Inline into ...
> 	(fold_single_bit_test): This and simplify.
Just to be clear, based on the NFC assumption, this is OK for the trunk.
jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/7] Use get_def_for_expr in fold_single_bit_test
  2023-05-20  2:14 ` [PATCH 3/7] Use get_def_for_expr in fold_single_bit_test Andrew Pinski
@ 2023-05-20  4:49   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-20  4:49 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/19/23 20:14, Andrew Pinski via Gcc-patches wrote:
> The code in fold_single_bit_test, checks if
> the inner was a right shift and improve the bitnum
> based on that. But since the inner will always be a
> SSA_NAME at this point, the code is dead. Move it over
> to use the helper function get_def_for_expr instead.
> 
> OK? Bootstrapped and tested on x86_64-linux.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (fold_single_bit_test): Use get_def_for_expr
> 	instead of checking the inner's code.
OK.
jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] Simplify fold_single_bit_test slightly
  2023-05-20  2:14 ` [PATCH 4/7] Simplify fold_single_bit_test slightly Andrew Pinski
@ 2023-05-20  4:51   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-20  4:51 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/19/23 20:14, Andrew Pinski via Gcc-patches wrote:
> Now the only use of fold_single_bit_test is in do_store_flag,
> we can change it such that to pass the inner arg and bitnum
> instead of building a tree. There is no code generation changes
> due to this change, only a decrease in GC memory that is produced
> during expansion.
> 
> OK? Bootstrapped and tested on x86_64-linux.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (fold_single_bit_test): Take inner and bitnum
> 	instead of arg0 and arg1. Update the code.
> 	(do_store_flag): Don't create a tree when calling
> 	fold_single_bit_test instead just call it with the bitnum
> 	and the inner tree.
OK.
jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/7] Simplify fold_single_bit_test with respect to code
  2023-05-20  2:14 ` [PATCH 5/7] Simplify fold_single_bit_test with respect to code Andrew Pinski
@ 2023-05-20  4:52   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-20  4:52 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/19/23 20:14, Andrew Pinski via Gcc-patches wrote:
> Since we know that fold_single_bit_test is now only passed
> NE_EXPR or EQ_EXPR, we can simplify it and just use a gcc_assert
> to assert that is the code that is being passed.
> 
> OK? Bootstrapped and tested on x86_64-linux.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (fold_single_bit_test): Add an assert
> 	and simplify based on code being NE_EXPR or EQ_EXPR.
OK.
jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/7] Use BIT_FIELD_REF inside fold_single_bit_test
  2023-05-20  2:14 ` [PATCH 6/7] Use BIT_FIELD_REF inside fold_single_bit_test Andrew Pinski
@ 2023-05-20  4:54   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-20  4:54 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/19/23 20:14, Andrew Pinski via Gcc-patches wrote:
> Instead of depending on combine to do the extraction,
> Let's create a tree which will expand directly into
> the extraction. This improves code generation on some
> targets.
> 
> OK? Bootstrapped and tested on x86_64-linux.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (fold_single_bit_test): Use BIT_FIELD_REF
> 	instead of shift/and.
OK.
jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] Expand directly for single bit test
  2023-05-20  2:14 ` [PATCH 7/7] Expand directly for single bit test Andrew Pinski
@ 2023-05-20  4:55   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-20  4:55 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/19/23 20:14, Andrew Pinski via Gcc-patches wrote:
> Instead of using creating trees to the expansion,
> just expand directly which makes the code a little simplier
> but also reduces how much GC memory will be used during the expansion.
> 
> OK? Bootstrapped and tested on x86_64-linux.
> 
> gcc/ChangeLog:
> 
> 	* expr.cc (fold_single_bit_test): Rename to ...
> 	(expand_single_bit_test): This and expand directly.
> 	(do_store_flag): Update for the rename function.
OK.

jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] Expand directly for single bit test
  2023-05-21 18:25 ` Andrew Pinski
  2023-05-21 18:45   ` Andrew Pinski
  2023-05-21 19:23   ` Jeff Law
@ 2023-05-21 23:01   ` David Edelsohn
  2 siblings, 0 replies; 21+ messages in thread
From: David Edelsohn @ 2023-05-21 23:01 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: Andrew Pinski, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

On Sun, May 21, 2023 at 11:25 AM Andrew Pinski <pinskia@gmail.com> wrote:

> On Sun, May 21, 2023 at 11:17 AM David Edelsohn via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Hi, Andrew
> >
> > Thanks for this series of patches to improve do_store_flag.
> Unfortunately
> > this specific patch in the series has caused a bootstrap failure on
> > powerpc-aix.  I bisected this failure to this specific patch. Note that I
> > am building as 32 bit, so this could be a specific issue about bit size.
> >
> >         * expr.cc (fold_single_bit_test): Rename to ...
> >         (expand_single_bit_test): This and expand directly.
> >         (do_store_flag): Update for the rename function.
>
> Did this include the fix I did for big-endian at
> r14-1022-g7f3df8e65c71e5 ? I had found that I broke big-endian last
> night with that patch and pushed the fix once I figured out what I did
> wrong.
> If you already tried post the fix, I will try to look into it as soon
> as possible.
>
>
The big-endian patch fixed the issue for Power also.

Thanks, David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] Expand directly for single bit test
  2023-05-21 18:25 ` Andrew Pinski
  2023-05-21 18:45   ` Andrew Pinski
@ 2023-05-21 19:23   ` Jeff Law
  2023-05-21 23:01   ` David Edelsohn
  2 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2023-05-21 19:23 UTC (permalink / raw)
  To: Andrew Pinski, David Edelsohn; +Cc: Andrew Pinski, GCC Patches



On 5/21/23 12:25, Andrew Pinski via Gcc-patches wrote:
> On Sun, May 21, 2023 at 11:17 AM David Edelsohn via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Hi, Andrew
>>
>> Thanks for this series of patches to improve do_store_flag.  Unfortunately
>> this specific patch in the series has caused a bootstrap failure on
>> powerpc-aix.  I bisected this failure to this specific patch. Note that I
>> am building as 32 bit, so this could be a specific issue about bit size.
>>
>>          * expr.cc (fold_single_bit_test): Rename to ...
>>          (expand_single_bit_test): This and expand directly.
>>          (do_store_flag): Update for the rename function.
> 
> Did this include the fix I did for big-endian at
> r14-1022-g7f3df8e65c71e5 ? I had found that I broke big-endian last
> night with that patch and pushed the fix once I figured out what I did
> wrong.
> If you already tried post the fix, I will try to look into it as soon
> as possible.
FWIW, the various failing hosts from yesterday in my tester have all 
returned to successful builds after the BE fixes.  m32r, iq2000, moxie, 
sh3eb, h8300.

There's a very reasonable chance the PPC bug is the same underlying issue.

Jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] Expand directly for single bit test
  2023-05-21 18:25 ` Andrew Pinski
@ 2023-05-21 18:45   ` Andrew Pinski
  2023-05-21 19:23   ` Jeff Law
  2023-05-21 23:01   ` David Edelsohn
  2 siblings, 0 replies; 21+ messages in thread
From: Andrew Pinski @ 2023-05-21 18:45 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Andrew Pinski, GCC Patches

On Sun, May 21, 2023 at 11:25 AM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Sun, May 21, 2023 at 11:17 AM David Edelsohn via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Hi, Andrew
> >
> > Thanks for this series of patches to improve do_store_flag.  Unfortunately
> > this specific patch in the series has caused a bootstrap failure on
> > powerpc-aix.  I bisected this failure to this specific patch. Note that I
> > am building as 32 bit, so this could be a specific issue about bit size.
> >
> >         * expr.cc (fold_single_bit_test): Rename to ...
> >         (expand_single_bit_test): This and expand directly.
> >         (do_store_flag): Update for the rename function.
>
> Did this include the fix I did for big-endian at
> r14-1022-g7f3df8e65c71e5 ? I had found that I broke big-endian last
> night with that patch and pushed the fix once I figured out what I did
> wrong.
> If you already tried post the fix, I will try to look into it as soon
> as possible.

I just re-read my message and I think it might have been confusing.
Last night I noticed the patch which you pointed out broke big-endian
targets, I pushed r14-1022-g7f3df8e65c71e5 as the fix. I am wondering
if your testing included this fix.
If yes then I will try to figure out the best way of figuring out how
I broke this target too.

Thanks,
Andrew

>
> Thanks,
> Andrew
>
> >
> >
> > Thanks, David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] Expand directly for single bit test
  2023-05-21 18:16 David Edelsohn
@ 2023-05-21 18:25 ` Andrew Pinski
  2023-05-21 18:45   ` Andrew Pinski
                     ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Andrew Pinski @ 2023-05-21 18:25 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Andrew Pinski, GCC Patches

On Sun, May 21, 2023 at 11:17 AM David Edelsohn via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi, Andrew
>
> Thanks for this series of patches to improve do_store_flag.  Unfortunately
> this specific patch in the series has caused a bootstrap failure on
> powerpc-aix.  I bisected this failure to this specific patch. Note that I
> am building as 32 bit, so this could be a specific issue about bit size.
>
>         * expr.cc (fold_single_bit_test): Rename to ...
>         (expand_single_bit_test): This and expand directly.
>         (do_store_flag): Update for the rename function.

Did this include the fix I did for big-endian at
r14-1022-g7f3df8e65c71e5 ? I had found that I broke big-endian last
night with that patch and pushed the fix once I figured out what I did
wrong.
If you already tried post the fix, I will try to look into it as soon
as possible.

Thanks,
Andrew

>
>
> Thanks, David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] Expand directly for single bit test
@ 2023-05-21 18:16 David Edelsohn
  2023-05-21 18:25 ` Andrew Pinski
  0 siblings, 1 reply; 21+ messages in thread
From: David Edelsohn @ 2023-05-21 18:16 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

Hi, Andrew

Thanks for this series of patches to improve do_store_flag.  Unfortunately
this specific patch in the series has caused a bootstrap failure on
powerpc-aix.  I bisected this failure to this specific patch. Note that I
am building as 32 bit, so this could be a specific issue about bit size.

	* expr.cc (fold_single_bit_test): Rename to ...
	(expand_single_bit_test): This and expand directly.
	(do_store_flag): Update for the rename function.


Thanks, David

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-05-21 23:01 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-20  2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski
2023-05-20  2:14 ` [PATCH 1/7] Move fold_single_bit_test to expr.cc from fold-const.cc Andrew Pinski
2023-05-20  4:43   ` Jeff Law
2023-05-20  2:14 ` [PATCH 2/7] Inline and simplify fold_single_bit_test_into_sign_test into fold_single_bit_test Andrew Pinski
2023-05-20  4:47   ` Jeff Law
2023-05-20  4:48   ` Jeff Law
2023-05-20  2:14 ` [PATCH 3/7] Use get_def_for_expr in fold_single_bit_test Andrew Pinski
2023-05-20  4:49   ` Jeff Law
2023-05-20  2:14 ` [PATCH 4/7] Simplify fold_single_bit_test slightly Andrew Pinski
2023-05-20  4:51   ` Jeff Law
2023-05-20  2:14 ` [PATCH 5/7] Simplify fold_single_bit_test with respect to code Andrew Pinski
2023-05-20  4:52   ` Jeff Law
2023-05-20  2:14 ` [PATCH 6/7] Use BIT_FIELD_REF inside fold_single_bit_test Andrew Pinski
2023-05-20  4:54   ` Jeff Law
2023-05-20  2:14 ` [PATCH 7/7] Expand directly for single bit test Andrew Pinski
2023-05-20  4:55   ` Jeff Law
2023-05-21 18:16 David Edelsohn
2023-05-21 18:25 ` Andrew Pinski
2023-05-21 18:45   ` Andrew Pinski
2023-05-21 19:23   ` Jeff Law
2023-05-21 23:01   ` David Edelsohn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).