This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] disentangle range_fold_*ary_expr into various pieces


On Mon, 7 Oct 2019, Aldy Hernandez wrote:

On 10/4/19 2:09 PM, Jeff Law wrote:

You're right.  Easier to refer to the before/after directly.  Sometimes
diffs just suck.

OK
jeff

In testing this patch in isolation from the non-zero canonicalization patch, I found one regression due to the fact that:

a) As discussed, two non-zero representations currently exist for unsigned ranges.

b) ipa-prop.c has it's own hacked up value_range structure (ipa_vr) which doesn't use any API. Since there is no agreed upon non-zero, range-ops can sometimes (correctly) create an unsigned [1,MAX], and ipa-prop.c is open-coding the check for a pointer non-zero to ~[0,0]. This seems like a latent bug.

I really have no idea, nor do I care (*), what we do with ipa-prop's lack of API. For now, I have implemented ipa_vr::nonzero_p(), and used it. When we agree on the non-zero normalization we can adjust this method if necessary.

+bool
+ipa_vr::nonzero_p (tree expr_type) const
+{
+  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))
+    return true;
+
+  unsigned prec = TYPE_PRECISION (expr_type);
+  return (type == VR_RANGE
+         && wi::eq_p (min, wi::one (prec))
+         && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));
+}

...

          else if (POINTER_TYPE_P (TREE_TYPE (ddef))
-                  && vr[i].type == VR_ANTI_RANGE
-                  && wi::eq_p (vr[i].min, 0)
-                  && wi::eq_p (vr[i].max, 0))
+                  && vr[i].nonzero_p (TREE_TYPE (ddef)))

Attached is the final adjusted patch I have committed to trunk.

I wonder why we would ever want to ask "is this interval the one that misses exactly the value 0" instead of "does this interval contain the value 0". I naively believe there shouldn't even be any API for the first question. Or if pointers really only have 2 possible intervals (besides varying and undefined), aka [0,0] and ~[0,0], using intervals seems like overkill for them...

--
Marc Glisse


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]