On Thu, 14 Apr 2016, Hurugalawadi, Naveen wrote:
I think we should handle at least INTEGER_CST and SSA_NAME
with VRP, and it seems natural to add a VRP check
The check should be added in the tree_single_nonzero_warnv_p
for SSA_NAME case for tree_expr_nonzero_p.
I think so.
However, for tree_expr_nonnegative_p, its been handled in a
different way.
Ah, indeed.
Should we combine this check with the existing one?
What do you mean exactly?
+ (if (!tree_expr_nonnegative_p (@1))
+ (cmp @2 @0))))))
Ideally, you would call tree_expr_nonpositive_p, except that that
function doesn't exist yet. So for now, I guess we
Would the tree_expr_nonpositive_p function be helpful for other cases
as well, I would try to add it if its useful.
My "ideally" was wrong. Ideally for this pattern, we would have a function
negative_p (since we don't want the zero value). But I don't know how
useful it would be elsewhere.
I was playing with this at some point, but I don't know if that's a good
idea...
(match negative_p
INTEGER_CST@0
(if (wi::lt_p (@0, 0, TYPE_SIGN (TREE_TYPE (@0))))))
(match negative_p
SSA_NAME@0
(if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
(with
{
wide_int min, max;
value_range_type rtype = get_range_info (@0, &min, &max);
}
(if (rtype == VR_RANGE && wi::lt_p (max, 0, TYPE_SIGN (TREE_TYPE (@0))))))))
Please find attached the modified patch as per the suggestions and
let me know if its fine?
Looks ok to me (not a reviewer), though you could now move the test
tree_expr_nonzero_p next to tree_expr_nonnegative_p (it is redundant for
the last case).
--
Marc Glisse
