On 11/11/20 10:45 AM, Richard Sandiford wrote:
Aldy Hernandez <al...@redhat.com> writes:
On 11/10/20 3:35 PM, Richard Sandiford wrote:
Aldy Hernandez <al...@redhat.com> writes:
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.
POLY_INT_CSTs aren't associated with separate types. They're just
values of normal integer type. Logically they come somewhere between an
INTEGER_CST and an SSA_NAME: they're not “as constant as” an INTEGER_CST
but not “as variable as” an SSA_NAME.
This means that they really could be treated as symbolic. The difficulty
is that POLY_INT_CSTs satisfy is_gimple_min_invariant, and the VRP code
fundamentally assumes that is_gimple_min_invariant on an integer type
means that the value must be an INTEGER_CST or an ADDR_EXPR. At one
point I'd posted a patch to add a new predicate specifically for that,
but Richard's response was that noone would remember to use it (which is
a fair comment :-)). So the current approach is instead to stop
POLY_INT_CSTs getting into the VRP system in the first place.
If the VRP code was rigorous about checking for INTEGER_CST before assuming
that something was an INTEGER_CST then no special handling of POLY_INT_CST
would be needed. But that's not the style that GCC generally follows and
trying to retrofit it now seems like fighting against the tide.
I agree, we should probably treat POLY_INTs as symbolics (or varying)
right at the setter and avoid dealing with them. Andrew may have a plan
for these later, but this will do for now.
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;
Very minor nit, sorry, but: formatting rules say that all checks should
be on one line or that there should be exactly one check per line.
Huh... I didn't know we had a rule for that, but it makes perfect sense.
I'll keep that in mind for future changes.
OK with that change. Thanks for the quick response in fixing this.
Thanks.
Aldy
