Re: [PATCH] Saturate overflows return from SCEV in ranger.

[Aldy Hernandez via Gcc-patches]

On 10/21/20 9:59 AM, Richard Biener wrote:

    /* Even for valid range info, sometimes overflow flag will leak in.
       As GIMPLE IL should have no constants with TREE_OVERFLOW set, we
       drop them.  */
    if (TREE_OVERFLOW_P (*min))
      *min = drop_tree_overflow (*min);
    if (TREE_OVERFLOW_P (*max))
      *max = drop_tree_overflow (*max);

Interesting.

If these values "leaked" in.  Should they have been fixed at the source,
instead of after the fact?  You mention below that every use of
TREE_OVERFLOW in the ME is a bug, should we clean them up before
arriving in gimple, or are there legitimate uses of it?

There are no legitimate uses in GIMPLE.  They are (ab-)used by
GENERIC folding for propagating overflow (also used in FE
diagnostics).  Generally the better way is to use wide_ints overflow
handling which also "sticks".

If there are no legitimate uses, perhaps we should drop them altogether 
as we go into GIMPLE??  I vaguely recall seeing them leak into 
value_range's.



and the code explicitly checks for overflow, doing range adjustments
accordingly.

Well, not all overflows are adjusted:

    /* Like in PR19590, scev can return a constant function.  */
    if (is_gimple_min_invariant (chrec))
      {
        *min = *max = chrec;
        return true;
      }

Are these min/max not adjusted for overflow by design, or is this an
oversight?

Ah, that's an oversight here.  And yes, "fixing" it in scalar evolution
analysis itself (dropping the flag there) would be best

Excellent.  I've pushed the patch below after testing it.

Thanks again.
Aldy

    Adjust overflow for invariants in bounds_of_var_in_loop.

    Invariants returned from SCEV can have TREE_OVERFLOW set.  Clear the
    overflow as we do with the rest of the values returned from this
    function.

    gcc/ChangeLog:

            * gimple-range.cc 
(gimple_ranger::range_of_ssa_name_with_loop_info):
            Remove TREE_OVERFLOW special case.
            * vr-values.c (bounds_of_var_in_loop): Adjust overflow for
            invariants.

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index b790d62d75f..c5520e0700b 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -1156,9 +1156,9 @@ gimple_ranger::range_of_ssa_name_with_loop_info 
(irange &r, tree name,
       // ?? We could do better here.  Since MIN/MAX can only be an
       // SSA, SSA +- INTEGER_CST, or INTEGER_CST, we could easily call
       // the ranger and solve anything not an integer.
-      if (TREE_CODE (min) != INTEGER_CST || TREE_OVERFLOW (min))
+      if (TREE_CODE (min) != INTEGER_CST)
        min = vrp_val_min (type);
-      if (TREE_CODE (max) != INTEGER_CST || TREE_OVERFLOW (max))
+      if (TREE_CODE (max) != INTEGER_CST)
        max = vrp_val_max (type);
       r.set (min, max);
     }
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index cc0ddca2bd3..7a0e70eab64 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -1844,7 +1844,7 @@ bounds_of_var_in_loop (tree *min, tree *max, 
range_query *query,
   if (is_gimple_min_invariant (chrec))
     {
       *min = *max = chrec;
-      return true;
+      goto fix_overflow;
     }

   if (TREE_CODE (chrec) != POLYNOMIAL_CHREC)
@@ -1964,6 +1964,7 @@ bounds_of_var_in_loop (tree *min, tree *max, 
range_query *query,
   else
     *min = init;

+ fix_overflow:
   /* Even for valid range info, sometimes overflow flag will leak in.
      As GIMPLE IL should have no constants with TREE_OVERFLOW set, we
      drop them.  */