On Fri, Dec 5, 2025 at 1:11 PM Daniel Henrique Barboza
<[email protected]> wrote:
>
> Hi,
>
> On 12/2/25 5:46 PM, Andrew Pinski wrote:
> > On Tue, Dec 2, 2025 at 12:11 PM Daniel Barboza
> > <[email protected]> wrote:
> >>
> >> Add a pattern to handle cases where we have an OP that is
> >> unconditionally being applied in the result of a gcond. In this case we
> >> can apply OP to both legs of the conditional. E.g:
> >>
> >> t = b ? 10 : 20;
> >> t = t + 20;
> >>
> >> becomes just:
> >>
> >> t = b ? 30 : 40
> >>
> >> PR 122608
> >>
> >> gcc/ChangeLog:
> >>
> >> * match.pd (`(c ? a : b) op d -> c ? (a op d) : (b op d)`): New
> >> pattern.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> * gcc.target/i386/pr110701.c: the pattern added is now folding
> >> an XOR into the ifcond, and the assembler isn't emitting an
> >> 'andl' anymore.
> >> * gcc.dg/torture/pr122608.c: New test.
> >>
> >> Signed-off-by: Daniel Barboza <[email protected]>
> >> ---
> >> gcc/match.pd | 10 +++
> >> gcc/testsuite/gcc.dg/torture/pr122608.c | 83 ++++++++++++++++++++++++
> >> gcc/testsuite/gcc.target/i386/pr110701.c | 2 +-
> >> 3 files changed, 94 insertions(+), 1 deletion(-)
> >> create mode 100644 gcc/testsuite/gcc.dg/torture/pr122608.c
> >>
> >> diff --git a/gcc/match.pd b/gcc/match.pd
> >> index f164ec59100..9650bbeada7 100644
> >> --- a/gcc/match.pd
> >> +++ b/gcc/match.pd
> >> @@ -6274,6 +6274,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >> && !expand_vec_cond_expr_p (TREE_TYPE (@1), TREE_TYPE
> >> (@0)))))
> >> (vec_cond @0 (op! @3 @1) (op! @3 @2)))))
> >>
> >> +
> >> +/* Non-vector comutative sink of binary ops to branches:
> > spelling: commutative . But these are not commutative (minus and div
> > are definitely not commutative) but rather just binary operators.
>
> I can't recall the context where this 'commutative' comment comes from. I'll
> remove it.
>
> >
> >> + (c ? a : b) op d --> c ? (a op d) : (b op d) */
> >> +(for op (plus minus mult bit_and bit_ior bit_xor
> >> + lshift rshift rdiv trunc_div ceil_div floor_div round_div
> >> exact_div
> >> + trunc_mod ceil_mod floor_mod round_mod min max)
> >
> > It might be useful to define a define_operator_list called tcc_binary
> > like what is done for tcc_comparison. And then just use that operator
> > list here.
> >
> >> + (simplify
> >> + (op (cond:s @0 @1 @2) @3)
> >> + (cond @0 (op! @1 @3) (op! @2 @3))))
> >
> > A few codes missing:
> > pointer_plus, pointer_diff, mult_highpart, lrotate, rrotate, complex
> > All comparison operators (tcc_comparison)
> >
> > Some internal functions missing:
> > COPYSIGN, POW, XORSIGN, HYPOT
> >
> > Oh I see you copied this exactly from the vector list and removed
> > tcc_comparison.
>
>
> I did some tests using current master to verify if these operators were
> already
> being reduced in this pattern or not. Turns out that most of the operators you
> mentioned are already being reduced without this patch.
>
> The following ops are already being reduced in master:
>
> - tcc_comparison
This is from:
```
#if GIMPLE
/* From fold_binary_op_with_conditional_arg handle the case of
rewriting (a ? b : c) > d to a ? (b > d) : (c > d) when the
compares simplify.
This should be after the boolean comparison simplification so
that it can remove the outer comparison before appling it to
the inner condtional operands. */
(for cmp (simple_comparison)
(simplify
(cmp:c (cond @0 @1 @2) @3)
/* Do not move possibly trapping operations into the conditional as this
pessimizes code and causes gimplification issues when applied late. */
(if (!FLOAT_TYPE_P (TREE_TYPE (@3))
|| !operation_could_trap_p (cmp, true, false, @3))
(cond @0 (cmp! @1 @3) (cmp! @2 @3)))))
#endif
```
Which you could merge your pattern with the above then. But it also
means you might need a commutative version and one for non-comutative
which has both versions. or you just support both for all.
> - pointer_diff
I tried pointer_diff and didn't get it optimized (note long is not
exactly right type, it needs to be a define but I can't remember the
define right now):
```
char *ptr[12];
__GIMPLE long pointer_diff_test(int a) {
_Bool b;
long t;
char *t1;
b = a > 0;
t1 = b ? _Literal(char *) & ptr[0] : _Literal(char *) & ptr[1];
t = t1 - _Literal(char *) & ptr[0];
return t;
}
```
> - mult_highpart
> - lrotate and rrotate
> - copysign
> - xorsign
> - pow
>
> As for HYPOT and 'complex', I can add them in the pattern but I didn't manage
> to
> find any example where they would reduce ... I'm either doing something wrong
> in
> the testcode or they aren't reducible via this pattern. In this case I'm not
> sure
> if adding them 'just in case' is a good idea.
>
>
>
> Thanks,
>
> Daniel
>
>
>
> >
> > Note pointer_plus is not likely reduce,
> > pointer_diff could:
> > (a ? &b[0] : &b[1]) - &b[0]
> > which should reduce to a ? 0 : 1;
> > complex could too:
> > COMPLEX_EXPR<a ? 2.0f : 4.0f, 5.0f> -> a ? COMPLEX_CST<2.0f, 5.0f>,
> > COMPLEX_CST<4.0f, 5.0f>
> >
> > Note these codes/internal functions can be added separately.
> >
> >
> >> +
> >> #if GIMPLE
> >> (match (nop_atomic_bit_test_and_p @0 @1 @4)
> >> (bit_and (convert?@4 (ATOMIC_FETCH_OR_XOR_N @2 INTEGER_CST@0 @3))
> >> diff --git a/gcc/testsuite/gcc.dg/torture/pr122608.c
> >> b/gcc/testsuite/gcc.dg/torture/pr122608.c
> >> new file mode 100644
> >> index 00000000000..f8058149e5a
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.dg/torture/pr122608.c
> >> @@ -0,0 +1,83 @@
> >> +/* { dg-do compile } */
> >> +/* { dg-options "-fgimple -fdump-tree-optimized" } */
> >> +/* { dg-skip-if "" { *-*-* } { "-O0" "-fno-fat-lto-objects" } { "" } } */
> >> +
> >> +#define F(OP,NAME) \
> >> + __GIMPLE int NAME##_test(int a) \
> >> + { _Bool b; \
> >> + int t; \
> >> + b = a > 0; \
> >> + t = b ? 20 : 40; \
> >> + t = t OP 11; \
> >> + return t; }
> >> +
> >> +F (+, plus)
> >> +F (-, minus)
> >> +F (*, mult)
> >> +F (|, bit_ior)
> >> +F (^, bit_xor)
> >> +F (/, div)
> >> +F (%, mod)
> >> +F (<<, lshift)
> >> +
> >> +__GIMPLE int test_and(int a)
> >> +{
> >> + _Bool b;
> >> + int t;
> >> + b = a > 0;
> >> + t = b ? 1 : 3;
> >> + t = t & 3;
> >> + return t;
> >> +}
> >> +
> >> +__GIMPLE int test_rshift(int a)
> >> +{
> >> + _Bool b;
> >> + int t;
> >> + b = a > 0;
> >> + t = b ? 2 : 8;
> >> + t = t >> 1;
> >> + return t;
> >> +}
> >> +
> >> +static int min (int a, int b)
> >> +{
> >> + return a < b ? a : b;
> >> +}
> >> +
> >> +static int max (int a, int b)
> >> +{
> >> + return a > b ? a : b;
> >> +}
> >> +
> >> +__GIMPLE int min_test(int a)
> >> +{
> >> + _Bool b;
> >> + int t;
> >> + b = a > 0;
> >> + t = b ? 2 : 4;
> >> + t = min (t, 3);
> >> + return t;
> >> +}
> >> +
> >> +__GIMPLE int max_test(int a)
> >> +{
> >> + _Bool b;
> >> + int t;
> >> + b = a > 0;
> >> + t = b ? 2 : 4;
> >> + t = max (t, 3);
> >> + return t;
> >> +}
> >> +
> >> +/* { dg-final { scan-tree-dump-times " \\+ " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times " \\* " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times " \\| " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times " \\^ " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times " & " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times " / " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times " % " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times " << " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times " >> " 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 0 "optimized" } } */
> >> +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "optimized" } } */
> >> diff --git a/gcc/testsuite/gcc.target/i386/pr110701.c
> >> b/gcc/testsuite/gcc.target/i386/pr110701.c
> >> index 3f2cea5c3df..8cd179c3b15 100644
> >> --- a/gcc/testsuite/gcc.target/i386/pr110701.c
> >> +++ b/gcc/testsuite/gcc.target/i386/pr110701.c
> >> @@ -8,5 +8,5 @@ void foo() {
> >> *c = d(340, b >= 0) ^ 3;
> >> }
> >>
> >> -/* { dg-final { scan-assembler "andl\[ \\t]\\\$340," } } */
> >> +/* { dg-final { scan-assembler-not "andl\[ \\t]\\\$340," } } */
> >
> > Changing this scan is most likely incorrect since it is looking for
> > wrong code. I suspect we should add a runtime testcase (that was
> > showing the original issue) and remove this test case instead.
> >
> > Thanks,
> > Andrew
> >
> >> /* { dg-final { scan-assembler-not "andw\[ \\t]\\\$340," } } */
> >> --
> >> 2.43.0
> >>
>
