On Wed, Jan 25, 2017 at 5:30 PM, Bin Cheng <bin.ch...@arm.com> wrote:
> Hi,
> As analyzed in PR71437, it's a missed PRE issue due to missed jump threading,
> and then due to inaccurate VRP information. In function
> extract_range_for_var_from_comparison_expr,
> we compute range for variable "a" under condition that comparison like "a <=
> limit"
> is true. It extracts limit's range information and set range [MIN,
> limit_vr->max] to var.
> This is inaccurate when limit_vr->max is MAX. In this case the final range
> computed
> is [MIN, MAX] which is VARYING. In fact, symbolic range info [MIN, limit] is
> better here.
> This patch fixes PR71437 by making such change. It also handles ">=" cases.
>
> Bootstrap and test on x86_64 and AArch64 finished. All tests are OK except
> test
> gcc.dg/tree-ssa/pr31521.c. I further investigated it and believe it's
> another missed
> optimization in VRP. Basically, operand_less_p is weak in handling symbolic
> value
> range. Given below value ranges:
> x: [1, INF+]
> a: [-INF, x - 1]
> b: [0, INF+]
> It doesn't know that "x - 1 < INF+" must be true, thus (intersect a b) is [0,
> x - 1].
> I believe there may be other places in which symbolic value range is not
> handled
> properly. So any comment?
The patch looks heuristically good though, for a < limit we chose
[MIN, limit - 1]
rather than [MIN, +INF - 1] which would be a non-varying integer range (and we
usually prefer those). So maybe restrict it to LE/GE_EXPR? That also fixes
pr31521.c but unfortunately regresses your new testcase again... so much for
heuristics ;)
You are correct about limitations in operand_less_p and improvements there
are of course welcome. Btw, I played with
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c (revision 244920)
+++ gcc/tree-vrp.c (working copy)
@@ -1663,7 +1663,12 @@ extract_range_for_var_from_comparison_ex
{
min = TYPE_MIN_VALUE (type);
- if (limit_vr == NULL || limit_vr->type == VR_ANTI_RANGE)
+ if (limit_vr == NULL || limit_vr->type == VR_ANTI_RANGE
+ || (limit_vr->type == VR_RANGE
+ && ((cond_code == LE_EXPR
+ && vrp_val_is_max (limit_vr->max)
+ && compare_values (limit_vr->min, limit_vr->max) != 0)
+ || is_overflow_infinity (limit_vr->max))))
max = limit;
else
{
@@ -1703,7 +1708,12 @@ extract_range_for_var_from_comparison_ex
{
max = TYPE_MAX_VALUE (type);
- if (limit_vr == NULL || limit_vr->type == VR_ANTI_RANGE)
+ if (limit_vr == NULL || limit_vr->type == VR_ANTI_RANGE
+ || (limit_vr->type == VR_RANGE
+ && ((cond_code == GE_EXPR
+ && vrp_val_is_min (limit_vr->min)
+ && compare_values (limit_vr->min, limit_vr->max) != 0)
+ || is_overflow_infinity (limit_vr->min))))
min = limit;
else
{
note the overflow_infinity case which would make us drop to varying below.
Thanks,
Richard.
> Thanks,
> bin
>
> 2017-01-24 Bin Cheng <bin.ch...@arm.com>
>
> PR tree-optimization/71437
> * tree-vrp.c (extract_range_for_var_from_comparison_expr): Prefer
> symbolic range form if limit has no useful range information.
>
> gcc/testsuite/ChangeLog
> 2017-01-24 Bin Cheng <bin.ch...@arm.com>
>
> PR tree-optimization/71437
> * gcc.dg/tree-ssa/pr71437.c: New test.
