* 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; 7+ 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] 7+ messages in thread
* Re: [PATCH 7/7] Expand directly for single bit test 2023-05-21 18:16 [PATCH 7/7] Expand directly for single bit test David Edelsohn @ 2023-05-21 18:25 ` Andrew Pinski 2023-05-21 18:45 ` Andrew Pinski ` (2 more replies) 0 siblings, 3 replies; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
* [PATCH 0/7] Improve do_store_flag @ 2023-05-20 2:14 Andrew Pinski 2023-05-20 2:14 ` [PATCH 7/7] Expand directly for single bit test Andrew Pinski 0 siblings, 1 reply; 7+ 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] 7+ 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 @ 2023-05-20 2:14 ` Andrew Pinski 2023-05-20 4:55 ` Jeff Law 0 siblings, 1 reply; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
end of thread, other threads:[~2023-05-21 23:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-21 18:16 [PATCH 7/7] Expand directly for single bit test 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 -- strict thread matches above, loose matches on Subject: below -- 2023-05-20 2:14 [PATCH 0/7] Improve do_store_flag Andrew Pinski 2023-05-20 2:14 ` [PATCH 7/7] Expand directly for single bit test Andrew Pinski 2023-05-20 4:55 ` Jeff Law
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).