On Mon, Aug 5, 2013 at 1:39 AM, Andrew Pinski <pins...@gmail.com> wrote: > On Mon, Aug 5, 2013 at 1:08 AM, Zhenqiang Chen <zhenqiang.c...@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 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. > > Then it would easier to detect this for all targets and easier to > remove from fold-const.c the removal of the short circuiting.
The other place where it make sense to do this optimization is the gimple combiner (right now that is really tree-ssa-forwprop.c). Though it would better sense if reassoc and ifcombine uses the same functions as the combiner (see my RFC which I don't have much time to work on right now). Thanks, Andrew Pinski > > Thanks, > Andrew Pinski > >> >> Thanks! >> -Zhenqiang >> >> ChangeLog >> 2013-08-05 Zhenqiang Chen <zhenqiang.c...@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.c...@arm.com> >> >> * gcc.dg/reassoc1.c: New testcase.