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: SSA range class and removal of VR_ANTI_RANGEs


Aldy Hernandez <aldyh@redhat.com> writes:
> On 06/20/2017 10:59 AM, Martin Sebor wrote:
>> On 06/20/2017 02:41 AM, Aldy Hernandez wrote:
>>> On 05/23/2017 03:26 PM, Martin Sebor wrote:
>>>> On 05/23/2017 04:48 AM, Aldy Hernandez wrote:
>>>
>>>> +  void Union (wide_int x, wide_int y);
>>>> +  bool Union (const irange &r);
>>>> +  bool Union (const irange &r1, const irange &r2);
>>>> +
>>>> +  // THIS = THIS ^ [X,Y].  Return TRUE if result is non-empty.
>>>> +  bool Intersect (wide_int x, wide_int y, bool readonly = false);
>>>> +  // THIS = THIS ^ R.  Return TRUE if result is non-empty.
>>>> +  // THIS = R1 ^ R2.  Return TRUE if result is non-empty.
>>>> +  bool Intersect (const irange &r1, const irange &r2, bool readonly =
>>>> false);
>>>> +  // Return TRUE if THIS ^ R will be non-empty.
>>>> +  bool Intersect_p (const irange &r)
>>>> +    { return Intersect (r, /*readonly=*/true); }
>>>>
>>>> I would suggest the following changes to Union, Intersect, and Not:
>>>>
>>>> 1) Define all three members without the readonly argument and
>>>>     returning irange& (i.e., *this).  The return value can be
>>>>     used wherever irange& is expected, and the is_empty() member
>>>>     function can be called on it to obtain the same result.  E.g.,
>>>>     Intersect A with B, storing the result in A:
>>>>
>>>>     irange A, B;
>>>>     if (A.Intersect (B).is_empty ()) { ... }
>>>>
>>>> 2) Add non-members like so:
>>>>
>>>>    irange range_union (const irange &lhs, const irange &rhs)
>>>>    {
>>>>      return irange (lhs).Union (rhs);
>>>>    }
>>>>
>>>>    and find out if the union of A or B is empty without modifying
>>>>    either argument:
>>>>
>>>>    irange A, B;
>>>>    if (range_union (A, B).is_empty ()) { ... }
>>>
>>> Perhaps we could provide an implicit conversion from irange to bool such
>>> that we could write:
>>>
>>> if (range_union (A, B)) { ... }
>>>
>>> as well as being able to write:
>>>
>>> if (!range_union (A, B).is_empty ()) { ... }
>>>
>>> That is, have range_union() return an irange as suggested, but have a
>>> bool overload (or whatever the C++ nomenclature is) such that converting
>>> an irange to a bool is interpreted as ``nitems != 0''.
>>>
>>> Is this acceptable C++ practice?
>> 
>> Implicit conversion to bool is a common way of testing validity
>> but I don't think it would be too surprising to use it as a test
>> for non-emptiness.  An alternative to consider is to provide
>> an implicit conversion to an unsigned integer (instead of
>> num_ranges()(*)) and have it return the number of ranges.  That
>> will make it possible to do the same thing as above while also
>> simplifying the API.
>
> I really like this.
>
> I have added an implicit conversion to unsigned that returns the number 
> of sub-ranges.  However, I have also left the empty_p() method for a 
> cleaner interface within the class itself.  It feels wrong to type "if 
> (!*this)" to represent emptiness within the class.

Conversions to bool and conversions to int both seem dangerous.
E.g. people might try "range1 | range2" as a shorthand for taking the
union of the ranges.  It'll compile but instead give the bitwise OR
of the two range counts.

How about instead having helper functions like intersect_p,
as for bitmaps?

Thanks,
Richard


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