On Fri, Dec 5, 2025 at 6:57 PM Andrew Pinski
<[email protected]> wrote:
>
> 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.
At least your new pattern should be next to this pattern for easier to find.
>
> > - 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
Sorry pressed send too soon.
I just checked and lrotate/rrotate is not done either:
```
#define F(OP,NAME) \
__GIMPLE int NAME##_test(int a) \
{ _Bool b; \
int t; \
b = a > 0; \
t = b ? 20 : 40; \
t = t OP 2; \
return t; }
F (__ROTATE_RIGHT, rotateright)
F (__ROTATE_LEFT, rotateleft)
```
Nor mult_highpart:
```
#define F(OP,NAME) \
__GIMPLE int NAME##_test(int a) \
{ _Bool b; \
int t; \
b = a > 0; \
t = b ? 30 : 40; \
t = t OP 0x7fffffff; \
return t; }
F (__MULT_HIGHPART, multhighpart)
```
> > - 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
> > >>
> >
