RE: [PATCH] Reassociate X == CST1 || X == CST2 if popcount (CST2 - CST1) == 1 into ((X - CST1) & ~(CST2 - CST1)) == 0

["Zhenqiang Chen" <zhenqiang dot chen at arm dot com>]

> -----Original Message-----
> From: Andrew Pinski [mailto:pinskia@gmail.com]
> Sent: Monday, August 05, 2013 4:40 PM
> To: Zhenqiang Chen
> Cc: GCC Patches
> Subject: Re: [PATCH] Reassociate X == CST1 || X == CST2 if popcount (CST2 -
> CST1) == 1 into ((X - CST1) & ~(CST2 - CST1)) == 0
> 
> On Mon, Aug 5, 2013 at 1:08 AM, Zhenqiang Chen
> <zhenqiang.chen@arm.com> wrote:
> > Hi
> >
> > The patch reassociates X == CST1 || X == CST2 if popcount (CST2 -
> > CST1) == 1 into ((X - CST1) & ~(CST2 - CST1)) == 0.
> >
> > Bootstrap on x86-64 and ARM chromebook.
> > No make check regression on x86-64 and panda board.
> >
> > For some targets/options, the "(X == CST1 || X == CST2)" might be
> > converted to "if (x == CST1) else if (x == CST2)" at the beginning. In
> > such case, the patch will not be executed. It is hard to write a
> > reliable testcase to detect the transformation. So the testcase does
> > not check the dump logs from
> > reassoc1 pass. It just checks the runtime result.
> >
> > Is it OK for trunk?
> Seems like a better place to put this is inside tree-ssa-ifcombine.c which

The patch is similar with the  code for 
     Optimize X == CST1 || X == CST2
     if popcount (CST1 ^ CST2) == 1 into
     (X & ~(CST1 ^ CST2)) == (CST1 & ~(CST1 ^ CST2))

Your "RFC: Gimple combine/folding interface" is a good design.
But before it, I think reusing code in tree-ssa-reassoc.c is the most easy way to implement it.

> handles the case where if(a || b) is split up into if(a) else if(b).
> Moving it into tree-ssa-ifcombine.c allows for code to be optimized which
> was written using the if(a) else if(b) form.

Yes. There might have opportunities to optimized "if(a) else if(b) form" for targets which LOGICAL_OP_NON_SHORT_CIRCUIT is FALSE.

If LOGICAL_OP_NON_SHORT_CIRCUIT is TRUE, "a || b" is converted to "a | b" in fold-const.c if they are simple operands.

Both reassoc and ifcombine optimize short circuiting. But they handle different scenarios. So I think we need both.

As you had mentioned, we need  enhance ifcombine to optimize "if(a) else if(b) form" using the similar methods from reassoc pass.
 
> Then it would easier to detect this for all targets and easier to remove from
> fold-const.c the removal of the short circuiting.

A unified interface as your RFC will be helpful for fold-const.c, tree-ssa-ifcombine.c and tree-ssa-reassoc.c.

If we can detect the optimization opportunity in fold_truth_andor, we do not need to split if(a || b) into if(a) else if(b) at all.

Thanks!
-Zhenqiang

> > ChangeLog
> > 2013-08-05  Zhenqiang Chen  <zhenqiang.chen@arm.com>
> >
> >         * tree-ssa-reassoc.c (optimize_range_tests): Reasociate
> >         X == CST1 || X == CST2 if popcount (CST2 - CST1) == 1 into
> >         ((X - CST1) & ~(CST2 - CST1)) == 0.
> >
> > testsuite/ChangeLog
> > 2013-08-05  Zhenqiang Chen  <zhenqiang.chen@arm.com>
> >
> >         * gcc.dg/reassoc1.c: New testcase.