* 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
* 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
* [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
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).