2011/5/19 Richard Guenther <richard.guent...@gmail.com>: > On Thu, May 19, 2011 at 3:30 PM, Kai Tietz <ktiet...@googlemail.com> wrote: >> 2011/5/19 Richard Guenther <richard.guent...@gmail.com>: >>> On Thu, May 19, 2011 at 3:08 PM, Kai Tietz <ktiet...@googlemail.com> wrote: >>>> 2011/5/19 Richard Guenther <richard.guent...@gmail.com>: >>>>> On Thu, May 19, 2011 at 2:59 PM, Kai Tietz <ktiet...@googlemail.com> >>>>> wrote: >>>>>> 2011/5/19 Richard Guenther <richard.guent...@gmail.com>: >>>>>>> On Thu, May 19, 2011 at 2:48 PM, Kai Tietz <ktiet...@googlemail.com> >>>>>>> wrote: >>>>>>>> Hello, >>>>>>>> >>>>>>>> This patch improves reassociation folding for comparision. It expands >>>>>>>> expressions within binary-AND/OR expression like (X | Y) == 0 to (X == >>>>>>>> 0 && Y == 0) >>>>>>>> and (X | Y) != 0 to (X != 0 || Y != 0). This is necessary to allow >>>>>>>> better reassociation >>>>>>>> on weak pre-folded logical expressions. This unfolding gets undone >>>>>>>> anyway later by pass, >>>>>>>> so no disadvantage gets introduced. >>>>>>>> Also while going through BB-list, it tries to do some little >>>>>>>> type-sinking for SSA sequences >>>>>>>> like "D1 = (type) bool1; D2 = (type) bool2; D3 = D1 & D2;' to 'D1 = >>>>>>>> bool1 & bool2; D2 = (type) D1;'. >>>>>>>> This folding has the advantage to see better through intermediate >>>>>>>> results with none-boolean type. >>>>>>>> The function eliminate_redundant_comparison () got reworded so, that >>>>>>>> doesn't break in all cases. >>>>>>>> It now continues to find duplicates and tries to find inverse variant >>>>>>>> (folded to constant). By this >>>>>>>> change we don't combine possible weak optimizations too fast, before >>>>>>>> we can find and handle >>>>>>>> inverse or duplicates. >>>>>>> >>>>>>> sinking casting belongs not here but instead to tree-ssa-forwprop. >>>>>>> I'm not sure that a != 0 | b != 0 is the better canonical variant than >>>>>>> a | b != 0 though. >>>>>>> >>>>>>> is_boolean_compatible_type_p looks like a strange remanent. >>>>>>> >>>>>>> Richard. >>>>>> >>>>>> Well, a | b != 0 is for sure more optimal, but for reassociation we >>>>>> need to see the unfolded variant temporary. This is necessary as >>>>>> fold-const can't see through SSA statements. But this kind of >>>>>> expansion should be reversed then by pass to the form (a | b) != 0 >>>>>> back. >>>>> >>>>> ? fold-const shouldn't deal with this at all as we are in gimple and in >>>>> SSA form. Surely re-association comes to play only with chains of >>>>> the above with more than two operands. >>>>> >>>>> Richard. >>>>> >>>>>> >>>>>> Regards, >>>>>> Kai >>>>>> >>>>> >>>> >>>> The issue you can see by testcase binop_tor4.c, as here are the >>>> intermediate variables d and e (with int type) are destroying the >>>> reassociation pass. This testcase for example needs this sinking. >>> >>> hoisting would work equally well >> >> Well, but just if then all operands in combined BIT_AND/OR block are >> getting hoisting. And well, there might be still some cases where we >> wouldn't find the equivalent. As hoisting leads to following >> sequences, eg: >> >> D1 = a != 0; >> D2 = b != 0; >> D3 = a == 0; >> D4 = b == 0; >> D5 = (long) D1 >> D6 = (long) D2 >> D7 = (long) D3 >> D8 = (long) D4 >> D9 = D5 & D6; >> D10 = D8 & D9 >> D11 = D9 & D10; >> >> which means that comparision folding will never will happen as the >> statement passed to fold algorithm is a cast to a comparison and not >> the comparison itself. So sinking looks more sane IMHO. > > The above is what you do.
No, I don't do this. Please see function sink_cast_and_expand function in patch. if (gimple_assign_cast_p (s1) && gimple_assign_cast_p (s2) && is_boolean_compatible_type_p (TREE_TYPE (gimple_assign_rhs1 (s1))) && is_boolean_compatible_type_p (TREE_TYPE (gimple_assign_rhs1 (s2))) && useless_type_conversion_p (TREE_TYPE (gimple_assign_rhs1 (s1)), TREE_TYPE (gimple_assign_rhs1 (s2)))) { gimple_stmt_iterator gsi; gimple sum; tree op1a, op1b, tmpvar; tmpvar = create_tmp_reg (TREE_TYPE (gimple_assign_rhs1 (s1)), NULL); add_referenced_var (tmpvar); sum = build_and_add_sum (tmpvar, gimple_assign_rhs1 (s1), gimple_assign_rhs1 (s2), code); op1 = gimple_get_lhs (sum); op1 = fold_convert (type, op1); op1a = gimple_assign_rhs1 (stmt); op1b = gimple_assign_rhs2 (stmt); gsi = gsi_for_stmt (stmt); gimple_assign_set_rhs_from_tree (&gsi, op1); update_stmt (stmt); remove_visited_stmt_chain (op1a); remove_visited_stmt_chain (op1b); ret = true; } The none-boolean cast get moved outer, not inner. Regards, Kai