On Fri, Oct 18, 2013 at 12:34 PM, Zhenqiang Chen <zhenqiang.c...@linaro.org> wrote: > On 18 October 2013 18:15, Richard Biener <richard.guent...@gmail.com> wrote: >> On Fri, Oct 18, 2013 at 12:06 PM, Zhenqiang Chen >> <zhenqiang.c...@linaro.org> wrote: >>> On 18 October 2013 17:53, Richard Biener <richard.guent...@gmail.com> wrote: >>>> On Fri, Oct 18, 2013 at 11:21 AM, Zhenqiang Chen >>>> <zhenqiang.c...@linaro.org> wrote: >>>>> On 18 October 2013 00:58, Jeff Law <l...@redhat.com> wrote: >>>>>> On 10/17/13 05:03, Richard Biener wrote: >>>>>>>>> >>>>>>>>> Is it OK for trunk? >>>>>>>> >>>>>>>> >>>>>>>> I had a much simpler change which did basically the same from 4.7 (I >>>>>>>> can update it if people think this is a better approach). >>>>>>> >>>>>>> >>>>>>> I like that more (note you can now use is_gimple_condexpr as predicate >>>>>>> for force_gimple_operand). >>>>>> >>>>>> The obvious question is whether or not Andrew's simpler change picks up >>>>>> as >>>>>> many transformations as Zhenqiang's change. If not are the things missed >>>>>> important. >>>>>> >>>>>> Zhenqiang, can you do some testing of your change vs Andrew P.'s change? >>>>> >>>>> Here is a rough compare: >>>>> >>>>> 1) Andrew P.'s change can not handle ssa-ifcombine-ccmp-3.c (included >>>>> in my patch). Root cause is that it does not skip "LABEL". The guard >>>>> to do this opt should be the same the bb_has_overhead_p in my patch. >>>> >>>> I think we want a "proper" predicate in tree-cfg.c for this, like maybe >>>> a subset of tree_forwarder_block_p or whatever it will end up looking >>>> like (we need "effectively empty BB" elsewhere, for example in >>>> vectorization, >>>> add a flag to allow a condition ending the BB and the predicate is done). >>>> >>>>> 2) Andrew P.'s change always generate TRUTH_AND_EXPR, which is not >>>>> efficient for "||". e.g. For ssa-ifcombine-ccmp-6.c, it will generate >>>>> >>>>> _3 = a_2(D) > 0; >>>>> _5 = b_4(D) > 0; >>>>> _6 = _3 | _5; >>>>> _9 = c_7(D) <= 0; >>>>> _10 = ~_6; >>>>> _11 = _9 & _10; >>>>> if (_11 == 0) >>>>> >>>>> With my patch, it will generate >>>>> >>>>> _3 = a_2(D) > 0; >>>>> _5 = b_4(D) > 0; >>>>> _6 = _3 | _5; >>>>> _9 = c_7(D) > 0; >>>>> _10 = _6 | _9; >>>>> if (_10 != 0) >>>> >>>> But that seems like a missed simplification in predicate combining >>>> which should be fixed more generally. >>>> >>>>> 3) The good thing of Andrew P.'s change is that "Inverse the order of >>>>> the basic block walk" so it can do combine recursively. >>>>> >>>>> But I think we need some heuristic to control the number of ifs. Move >>>>> too much compares from >>>>> the inner_bb to outer_bb is not good. >>>> >>>> True, but that's what fold-const.c does, no? >>> >>> Based on current fold-const, we can not generate more than "two" >>> compares in a basic block. >> >> I think you are wrong as fold is invoked recursively on a && b && c && d. > > Please try to compile it with -fdump-tree-gimple. It will be broken into > > if (a && b) > if (c && d)
Indeed - it seems to handle all kinds of series but not the incremental one that appears after the first folding - (a truth-and b) truth-andif c. >>> But if ifs can be combined recursively in ifcombine, we might generate >>> many compares in a basic block. >> >> Yes, we can end up with that for the other simplifications done in >> ifcombine as well. Eventually there needs to be a cost model for this >> (use edge probabilities for example, so that if you have profile data >> you never combine cold blocks). >> >> Any takers? > > I will take it. > > Thanks! > -Zhenqiang > >> Richard. >> >>>>> 4) Another good thing of Andrew P.'s change is that it reuses some >>>>> existing functions. So it looks much simple. >>>> >>>> Indeed - that's what I like about it. >>>> >>>>>>> >>>>>>> With that we should be able to kill the fold-const.c transform? >>>>>> >>>>>> That would certainly be nice and an excellent follow-up for Zhenqiang. >>>>> >>>>> That's my final goal to "kill the fold-const.c transform". I think we >>>>> may combine the two changes to make a "simple" and "good" patch. >>>> >>>> Thanks, >>>> Richard. >>>> >>>>> Thanks! >>>>> -Zhenqiang
