On Tue, May 23, 2017 at 12:48 PM, Aldy Hernandez <al...@redhat.com> wrote:
> Howdy!
>
> For Andrew's upcoming on-demand range work I have created a range class for
> use in his engine. Currently, the range class is only for SSA integers, but
> there is no reason why we couldn't adapt this for RTL or non-integer types
> at a later time.
>
> The class can live outside of his work, as can be demonstrated by the
> attached patch. With it, I was able to rewrite the post-VRP range
> information to use this class and get rid of VR_ANTI_RANGE throughout the
> compiler. A VR_ANTI_RANGE of ~[5,10] becomes [-MIN,4][11,+MAX].
>
> The internal structure of VRP is unchanged. I have only changed the outward
> facing structures.
>
> A good example of how much cleaner using an actual range rather than
> VR_ANTI_RANGEs is the change to get_size_range() in the attached patch. We
> remove an assortment of contortions into:
>
> + /* Remove negative numbers from the range. */
> + irange positives;
> + range_positives (&positives, exptype, allow_zero ? 0 : 1);
> + if (positives.Intersect (*ir))
> + {
> + /* Remove the unknown parts of a multi-range.
> + This will transform [5,10][20,MAX] into [5,10]. */
> + if (positives.num_ranges () > 1
> + && positives.upper_bound () == wide_int (TYPE_MAX_VALUE
> (exptype)))
> + positives.remove_range (positives.num_ranges () - 1);
> +
> + range[0] = wide_int_to_tree (exptype, positives.lower_bound ());
> + range[1] = wide_int_to_tree (exptype, positives.upper_bound ());
> + }
> + else
> + {
> + /* If removing the negative numbers didn't give us anything
> + back, the entire range was negative. Leave things as they
> + were, and let the caller sort it out. */
> + range[0] = wide_int_to_tree (exptype, min);
> + range[1] = wide_int_to_tree (exptype, max);
> }
>
> A few notes:
>
> 1. There are still two uses of VR_ANTI_RANGEs left after this patch: ip-cp.c
> and ipa-prop.c. I didn't look too hard at these two passes, as they are
> using `struct value_range' which should only have been used *before* VRP
> finishes. I can work on this as a follow-up.
So my ultimate goal was to somehow make the range operations in VRP
(union, intersect, unary and binary ops) have an API that can be easily
used with both the internal VRP lattice ranges and the SSA name associated
ranges. Now that we have C++ we could either use some kind of adaptors
or make them templates -- or something else.
> 2. I have not included ChangeLog entries, as I would hate to write them, and
> have the API or significant things change from under me as part of the
> review. I can certainly cobble the ChangeLog entries as soon as/if we agree
> that this is an acceptable approach.
>
> 3. There are lots of unit tests to maintain sanity. The cast ones in
> particular will definitely need refinement as Andrew's work stresses the
> class.
>
> The attached patch passes bootstrap and tests.
>
> I have also benchmarked an assortment of .ii files (from a previous
> bootstrap) with no noticeable difference in -O2 compilation time. So, I
> would prefer to tackle any micro-optimizations either as a follow-up or if
> it actually becomes noticeable--. Yeah, yeah, I know. Wishful thinking ;-).
+ // If we will overflow, don't bother. This will handle unsigned
+ // underflow which doesn't set the overflow bit.
+ //
+ // Note: Perhaps all these &ovf checks are unecessary since we
+ // are manually checking for overflow with the if() below.
no multi-line // comments please. What's wrong with /* ... */?
+// Convert the current range in place into a range of type NEW_TYPE.
+// The type of the original range is changed to the new type.
+
+void
+irange::cast (tree new_type)
+{
+ if (!n)
so this duplicates VRP unary op with NOP/CONVERT_EXPR.
+void
+irange::Union (wide_int x, wide_int y)
+{
why capital U?
>
> Fire away!
> Aldy
>
> p.s. Please refer any comments as to why we need the class, or why we need
> on-demand range information to Andrew :-P.
