On 11/26/24 3:49 AM, Jovan Vukic wrote:
Thank you for the feedback on the v1 patch.
As requested, I added detailed tests for various signed and unsigned integer
types in the test file bitops-11.c. I also included more complex expressions to
observe how everything behaves at the GIMPLE level and added vector test
examples as well. Since vector expressions are matched on (minus @0 1) instead
of (plus @0 -1), I added a simplification for the minus case in match.pd.
Additionally, I introduced simplifications for the expressions (a - 1) & -a,
(a - 1) | -a, and (a - 1) ^ -a to 0, -1, and -1, respectively, in
simplify-rtx.cc.
For each of the three expressions, I added two if statements. The first matches
the typical (BIT_OP (plus (A -1)) (neg A)), while the second recognizes the
presence of a SUBREG within the RTL expression. For example, when A is of type
short, the second if statement is triggered. I didn't observe any issues with
match.pd missing any simplifications, but if that happens, the code in
simplify-rtx.cc should help.
Bootstrapped and tested on x86-linux-gnu with no regressions.
2024-11-26 Jovan Vukic<jovan.vu...@rt-rk.com>
gcc/ChangeLog:
* match.pd: New pattern.
* simplify-rtx.cc (simplify_context::simplify_binary_operation_1): New
code to handle (a - 1) & -a, (a - 1) | -a and (a - 1) ^ -a.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/bitops-11.c: New test.
CONFIDENTIALITY: The contents of this e-mail are confidential and intended only
for the above addressee(s). If you are not the intended recipient, or the
person responsible for delivering it to the intended recipient, copying or
delivering it to anyone else or using it in any unauthorized manner is
prohibited and may be unlawful. If you receive this e-mail by mistake, please
notify the sender and the systems administrator atstraym...@rt-rk.com
immediately.
patch.patch
---
gcc/match.pd | 16 +++
gcc/simplify-rtx.cc | 87 ++++++++++++++++
gcc/testsuite/gcc.dg/tree-ssa/bitops-11.c | 116 ++++++++++++++++++++++
3 files changed, 219 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/bitops-11.c
diff --git a/gcc/match.pd b/gcc/match.pd
index 0ac5674f24b..c85d4b9ae6c 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1472,6 +1472,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(bit_and:c @0 (bit_not (bit_xor:c @0 @1)))
(bit_and @0 @1))
+/* Transform:
+ (a - 1) & -a -> 0.
+ (a - 1) | -a -> -1.
+ (a - 1) ^ -a -> -1. */
+(for bit_op (bit_ior bit_xor bit_and)
+ (simplify
+ (bit_op:c (plus @0 integer_minus_onep@1) (negate @0))
+ (if (bit_op == BIT_AND_EXPR)
+ { build_zero_cst (type); }
+ { build_minus_one_cst (type); }))
+ (simplify
+ (bit_op:c (minus @0 integer_onep@1) (negate @0))
+ (if (bit_op == BIT_AND_EXPR)
+ { build_zero_cst (type); }
+ { build_minus_one_cst (type); })))
I'm a little surprised the second form matches anything since I would
expect canonicalization to turn the second form into the first. But may
we're not as good at canonicalizing trees as we are RTL.
+
/* a & (a == b) --> a & b (boolean version of the above). */
(simplify
(bit_and:c @0 (nop_convert? (eq:c @0 @1)))
diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index 893c5f6e1ae..514d21c6ef5 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -3530,6 +3530,35 @@ simplify_context::simplify_binary_operation_1 (rtx_code
code,
&& GET_MODE_CLASS (mode) != MODE_CC)
return CONSTM1_RTX (mode);
+ /* (ior (plus (A -1)) (neg A)) is -1. */
Nit. Space between the operator "-" and the operand "1".
+ if (((GET_CODE (op1) == NEG
+ && GET_CODE (op0) == PLUS
+ && XEXP (op0, 1) == constm1_rtx)
Most likely constm1_rtx should be CONSTM1_RTX (mode). Similarly for the
other instances.
+ || (GET_CODE (op0) == NEG
+ && GET_CODE (op1) == PLUS
+ && XEXP (op1, 1) == constm1_rtx))
+ && rtx_equal_p (XEXP (op0, 0), XEXP (op1, 0))
+ && !side_effects_p (XEXP (op0, 0))
+ && !side_effects_p (XEXP (op1, 0)))
If the RTXs are equal, then the second call to !side_effects_p isn't needed.
Given the duplication of RTL scanning across the AND, XOR, IOR cases,
wouldn't it make sense to factor the tests into a new function and just
call it from all 3 locations?
Overall it looks pretty good. I'm a bit concerned about the SUBREG
case, but I haven't come up with any scenarios where it doesn't work yet.
Jeff
