On 11/10/20 3:35 PM, Richard Sandiford wrote:
Aldy Hernandez <al...@redhat.com> writes:
(actually I can see 3245 ICEs on aarch64)
Can you fix it?
Sure can.
Richard, I seem to have incorrectly removed the early exit for varying,
and that affected the changes you made for poly ints. Is there any
reason we can't just exit and set varying without checking for kind !=
VR_VARYING?
No reason, it seemed more natural to drop to a lower kind with the old
code :-) (But not with the new code.)
But it isn't obvious to me why the code is now structed the way it is.
if (POLY_INT_CST_P (min) || POLY_INT_CST_P (max))
{
set_varying (TREE_TYPE (min));
return;
}
// Nothing to canonicalize for symbolic ranges.
if (TREE_CODE (min) != INTEGER_CST
|| TREE_CODE (max) != INTEGER_CST)
{
m_kind = kind;
m_base[0] = min;
m_base[1] = max;
m_num_ranges = 1;
return;
}
swap_out_of_order_endpoints (min, max, kind);
if (kind == VR_VARYING)
{
set_varying (TREE_TYPE (min));
return;
}
Why do we want to check “min” and “max” being INTEGER_CST before “kind”
being VR_VARYING, and the potentially record VR_VARYING with specific
bounds? And why do we want to swap the “min” and “max” before checking
whether “kind” is VR_VARYING (when we'll then drop the min and max anyway)?
I think this would benefit from a bit more commentary at least.
The main idea was to shorten the code and avoid having to exit due to
varying at various points (early and after the operands had been
swapped). But yes, it took more cycles.
BTW, VR_VARYING does get specific bounds, by design. What could've
happened in the code was someone feeding VR_VARYING with non-integer
bounds. This would've built an invalid VR_VARYING.
How about this (on top of the previous patch which I already pushed to
un-break aarch64)?
Aldy
p.s. If POLY_INT_CST_P are not supported in ranges, but are
INTEGRAL_TYPE_P, perhaps we should also tweak irange::supports_type_p so
it doesn't leak in anywhere else.
gcc/ChangeLog:
* value-range.cc (irange::set): Early exit on VR_VARYING.
---
gcc/value-range.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index b7ccba010e4..3703519b03a 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -249,7 +249,8 @@ irange::set (tree min, tree max, value_range_kind kind)
return;
}
- if (POLY_INT_CST_P (min) || POLY_INT_CST_P (max))
+ if (kind == VR_VARYING
+ || POLY_INT_CST_P (min) || POLY_INT_CST_P (max))
{
set_varying (TREE_TYPE (min));
return;
--
2.26.2
