Re: SSA range class and removal of VR_ANTI_RANGEs

[Andrew MacLeod <amacleod at redhat dot com>]

On 05/23/2017 03:26 PM, Martin Sebor wrote:
> On 05/23/2017 04:48 AM, Aldy Hernandez wrote:
> > Howdy!
> +typedef class irange *irange_p;
>
> FWIW, I find pointer typedefs more trouble than worth.  They
> obscure the fact that they are pointers and cannot be made const
> by adding the const qualifier.  E.g., this:
>
>   void foo (const irange_p);
>
> doesn't declare foo to take a pointer to a const range as one
> might hope but rather a const pointer to a non-const range.

yeah, the trend is to expose pointers, like 'gimple *' does. extracting 
types from trees into 'ttype *' was doing the same thing.  we should be 
able to punt on irange_p.

> +
> +public:
> +  irange () { type = NULL_TREE; n = 0; }
> +  irange (tree t);
>
> Should the range be implicitly convertible from any tree or would
> it better if it required explicit conversion (i.e., should the ctor
> be declared explicit)?  The usual rule of thumb is to err on the
> side of safety to avoid accidentally creating objects of the class).
the original idea was to allow it to be created from either
  a) an ssa_name, which would return what global range we know about 
the name,
  b) a type , and set it to the range_for_type().
and it would gcc_assert fail on any other kind.

a) may or may not be a good idea or useful long term... it might be 
better to simply always go through a central ssa_name range server 
class, which doesnt quite exist yet.  That would subsume this.
b) some day (haha), this would be statically types as irange (ttype *) 
rather than tree for types.    It could also be provided trough an 
external routine.. irange_for_type (tree type)

neither are probably needed

> +  irange (tree t, wide_int lbound, wide_int ubound,
> +      irange_type rt = RANGE_PLAIN);
> +  irange (const irange &r);
> +
> +  void set_range (tree t);
> +  void set_range (tree t, wide_int lbound, wide_int ubound,
> +          irange_type rt = RANGE_PLAIN);
> +
> +  bool overflow_p () { return overflow && !TYPE_OVERFLOW_WRAPS (type);
>
> Suggest to declare this function const.

I figured this would come up.  We ought to properly constify the entire 
class where appropriate. the more code that gets written using 
un-constified code, the harder it is to go back an constify it.


> PS For the names, have you considered using the bitwise operators?
> I.e., operator| for union, operator& for intersection, and operator~
> for not (aka invert).


I'd be more tempted to follow the bitmap model and ( how you mentioned 
earlier I think) have friend routines for range_intersect(), 
range_invert(), range_union().  especially since we want 2 variations. 
one which takes a single parameter and works into 'this',  and another 
variation that builds a new range from 2 parametered ranges .

I feel overloading those operators would make the code a little less 
readable?  at least to me.

Andrew