On Wed, Nov 4, 2015 at 12:18 PM, Marc Glisse <marc.gli...@inria.fr> wrote: > On Wed, 4 Nov 2015, Hurugalawadi, Naveen wrote: > >>>> I thought we were mostly using the 'convert?' >>>> and tree_nop_conversion_p on integers
Yes, on floats they shouldn't be used. >> >> Done. Cleared all instances of convert which are not required. >> However, I am still confused about the use of "convert" in match >> and simplify. > > > It could be that I am wrong, you'll have to wait for Richard's comments > anyway. The one place where a convert could be useful is: > (div (convert? (bit_and @0 INTEGER_CST@1)) INTEGER_CST@2) > but I didn't check if we want it (it would probably at least require > (convert @0) instead of plain @0 in the result so the division and the shift > are done with the same signedness). > >>>> So all patterns looking at arg[01] are handling nop-conversions on their >>>> outermost operands while those looking at op[01] do not. >> >> >> These patterns are looking at arg[01] rather than op[01] ? > > > Might be a case where it doesn't really matter which one you look at, and > the easiest one in fold-const may not be the same as in match.pd. > > +/* Convert (A/B)/C to A/(B*C) */ > +(simplify > + (rdiv (rdiv @0 @1) @2) > + (if (flag_reciprocal_math) > > I would indent this line the same as the one above. Yep. > + (rdiv @0 (mult @1 @2)))) > + > +/* Convert A/(B/C) to (A/B)*C */ > +(simplify > + (rdiv @0 (rdiv @1 @2)) > + (if (flag_reciprocal_math) > + (mult (rdiv @0 @1) @2))) > > I believe you might want a :s on the inner rdiv (?). > Note that you can factor out the test on flag_reciprocal_math so you write > it only once for 2 patterns. Please use :s on both inner rdiv in both patterns. With that the two patterns are ok. +/* Convert C1/(X*C2) into (C1/C2)/X */ +(simplify + (rdiv (REAL_CST@0) (mult @1 REAL_CST@2)) Omit the parens around REAL_CST@0 + (if (flag_reciprocal_math) + (with + { tree tem = const_binop (RDIV_EXPR, type, @0, @2); } + (if (tem) + (rdiv { tem; } @1))))) with that the pattern is ok. > > I don't really remember what the tests !TYPE_UNSIGNED (type) and > tree_int_cst_sgn are for in the other pattern, but since you are only moving > the transformation... +/* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */ +(for div (exact_div trunc_div) + (simplify + (div (bit_and @0 INTEGER_CST@1) INTEGER_CST@2) + (if (!TYPE_UNSIGNED (type) && integer_pow2p (@2) + && tree_int_cst_sgn (@2) > 0 + && wi::add (@2, @1) == 0) + (rshift @0 { build_int_cst (integer_type_node, wi::exact_log2 (@2)); })))) the TYPE_UNSIGNED test is because right shift of negative values is undefined, so is a shift with a negative value. I believe we can safely handle conversions here just like fold-const.c does with (div (convert? (bit_and @0 INTEGER_CST@1) INTEGER_CST@2) (if (tree_nop_conversion_p (type, TREE_TYPE (@0))) ... With that the pattern looks ok to me. Richard. > > -- > Marc Glisse