On Mon, 1 Dec 2014, Joseph Myers wrote: > On Mon, 1 Dec 2014, Richard Biener wrote: > > > +/* Combine two successive divisions. */ > > +(for div (trunc_div ceil_div floor_div round_div exact_div) > > This doesn't seem correct for all kinds of division and signedness of > arguments. > > TRUNC_DIV_EXPR (C division) and EXACT_DIV_EXPR should be OK regardless. > But for CEIL_DIV_EXPR and FLOOR_DIV_EXPR, in ((a / b) / c) you need c to > be positive so that both roundings are in the same direction (consider > e.g. 3 FLOOR_DIV -2 FLOOR_DIV -2 == 1, but 3 FLOOR_DIV 4 == 0). And for > ROUND_DIV_EXPR, it can be incorrect whatever the signs (e.g. 2 ROUND_DIV 3 > ROUND_DIV 2 == 1, but 2 ROUND_DIV 6 == 0). > > I'm not sure if these forms of division actually occur in places where > this could cause a problem, but it does look like Ada may enable you to > generate ROUND_DIV_EXPR.
Hmm. I thought I was following what extract_muldiv_1 does (but of course that function is quite hard to follow). case TRUNC_DIV_EXPR: case CEIL_DIV_EXPR: case FLOOR_DIV_EXPR: case ROUND_DIV_EXPR: case EXACT_DIV_EXPR: ... /* If these are the same operation types, we can associate them assuming no overflow. */ if (tcode == code) { bool overflow_p = false; bool overflow_mul_p; signop sign = TYPE_SIGN (ctype); wide_int mul = wi::mul (op1, c, sign, &overflow_mul_p); overflow_p = TREE_OVERFLOW (c) | TREE_OVERFLOW (op1); if (overflow_mul_p && ((sign == UNSIGNED && tcode != MULT_EXPR) || sign == SIGNED)) overflow_p = true; if (!overflow_p) return fold_build2 (tcode, ctype, fold_convert (ctype, op0), wide_int_to_tree (ctype, mul)); but yeah, I see you are correct. I'll restrict the pattern to TRUNC_DIV_EXPR and EXACT_DIV_EXPR which are the only ones I've ever seen generated from C. I don't know how to generate testcases for the other division opcodes - eventually we should have builtins just for the sake of being able to generate testcases... Committed as obvoious. Thanks, Richard. 2014-12-02 Richard Biener <rguent...@suse.de> * match.pd: Restrict division combining to trunc_div and exact_div. Index: gcc/match.pd =================================================================== --- gcc/match.pd (revision 218258) +++ gcc/match.pd (working copy) @@ -129,8 +129,9 @@ (define_operator_list inverted_tcc_compa && TYPE_UNSIGNED (type)) (trunc_div @0 @1))) -/* Combine two successive divisions. */ -(for div (trunc_div ceil_div floor_div round_div exact_div) +/* Combine two successive divisions. Note that combining ceil_div + and floor_div is trickier and combining round_div even more so. */ +(for div (trunc_div exact_div) (simplify (div (div @0 INTEGER_CST@1) INTEGER_CST@2) (with {