On Tue, May 27, 2014 at 11:25 AM, Eric Botcazou <ebotca...@adacore.com> wrote: >> + tree new_const >> + = fold_build2_loc (loc, reverse_op, TREE_TYPE (arg1), const2, >> const1); >> >> /* If the constant operation overflowed this can be >> simplified as a comparison against INT_MAX/INT_MIN. */ >> - if (TREE_CODE (lhs) == INTEGER_CST >> - && TREE_OVERFLOW (lhs)) >> + if (TREE_OVERFLOW (new_const)) >> >> well, either use int_const_binop above or retain the check (or use >> TREE_OVERFLOW_P). Bonus points for using wide-ints here >> and not relying on TREE_OVERFLOW. > > The check is useless (you get either NULL_TREE or INTEGER_CST here) but I'll > use int_const_binop. > >> + /* Transform comparisons of the form X - Y CMP 0 to X CMP Y. */ >> + if (TREE_CODE (arg0) == MINUS_EXPR >> + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1)) >> >> any good reason for using TYPE_OVERFLOW_UNDEFINED on the >> type of arg1 instead on the type of the MINUS (yes, they should >> match, but it really looks odd ... the overflow of the minus has to be >> undefined). > > For the sake of consistency with the X +- C1 CMP C2 case just above, but I can > change both. > >> Also for EQ_EXPR and NE_EXPR the transform is >> fine even when !TYPE_OVERFLOW_UNDEFINED (and we seem >> to perform it already somewhere). Please look where and try to >> add the undefined overflow case to it. > > Yes, but it's the same for the X +- C1 CMP C2 case, i.e. there are specific > cases for X +- C1 EQ/NE C2 and X - Y EQ/NE 0 in fold_binary, so I'm not sure > what you're asking.
I'm asking to merge them (move them to fold_comparison). >> As for the VRP pieces I don't understand what is missing here >> (well, compare_range_with_value and/or compare_values might >> not handle this case? then better fix that to improve symbolic >> range handling in general?). Also I have a longstanding patch >> in my tree that does > > Yes, there is an explicit non-handling of symbolic ranges for PLUS_EXPR and > MINUS_EXPR in VRP (extract_range_from_binary_expr_1) and the patch works > around it by propagating the code instead of the ranges, which is far easier > and sufficient here. If you think that the way to go is to handle symbolic > ranges for PLUS_EXPR and MINUS_EXPR instead, fine with me, I can try. Yeah, it would be nice to see some support. The most interesting cases will be symbolic-singleton +- CST where the offset shrinks a constant offset in a symbolic A +- CST (thus we don't get into any overflow issues). Thus handling [a + 1, a + 1] - [1, 1] -> [a, a] for example. We get the offsetted singleton symbolic ranges from conditional asserts a lot. Thanks, Richard. > -- > Eric Botcazou
