Re: [PATCH 3/4 v3][PR 67328] Added bool conversion for wide_ints

[Yuri Gribov <tetra2005@gmail.com>]

On Tue, May 30, 2017 at 4:35 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> Yuri Gribov <tetra2005@gmail.com> writes:
>> On Tue, May 30, 2017 at 11:35 AM, Richard Sandiford
>> <richard.sandiford@linaro.org> wrote:
>>> Yuri Gribov <tetra2005@gmail.com> writes:
>>>> On Tue, May 30, 2017 at 7:35 AM, Richard Sandiford
>>>> <richard.sandiford@linaro.org> wrote:
>>>>> Yuri Gribov <tetra2005@gmail.com> writes:
>>>>>> From 330209f721a598ec393dcb5d62de3457ee282153 Mon Sep 17 00:00:00 2001
>>>>>> From: Yury Gribov <tetra2005@gmail.com>
>>>>>> Date: Fri, 26 May 2017 07:53:10 +0100
>>>>>> Subject: [PATCH 3/4] Added bool conversion for wide_ints.
>>>>>>
>>>>>> gcc/
>>>>>> 2017-05-26  Yury Gribov  <tetra2005@gmail.com>
>>>>>>
>>>>>>       * wide-int.cc (wi::zero_p_large): New method.
>>>>>>       * wide-int.h (wi::zero_p): New method.
>>>>>
>>>>> Do you still need this bit?  It looks like it isn't used by the other
>>>>> parts of the series.
>>>>>
>>>>> The idea was that wi::eq_p (x, 0) (or just x == 0, if x is a
>>>>> wide-int-based type) is supposed to be as fast as a dedicated zero check.
>>>>> It'd be OK to have a helper function anyway, but it should probably be
>>>>> defined using wi::eq_p.
>>>>>
>>>>> The zero_p_large fallback can never return true, since a zero of
>>>>> any precision will have a length of 1.
>>>>
>>>> Thanks Richard, I'll update the patch. The bool check is used in
>>>> successive patch (4/4), in
>>>>      widest_int mask = wi::to_widest (@2);
>>>>      bool mask_all_ones_p = !(mask & (mask + 1));
>>>
>>> Ah, OK.  That's equivalent to mask == -1 (or wi::eq_p (@2, -1), to avoid
>>> the temporary).
>>
>> Hm, is it? Current check ensures that N consecutive LSBs are set, not
>> that all bits are set. Perhaps variable name should be changed to
>> reflect this better.
>
> Sorry, yeah, was going off the variable name rather than what the
> test actually did...
>
>>> I think it'd be better to use one of those instead.
>>> There's an argument that if ! is defined, it should return an integer
>>> of the same precision as the argument.
>>
>> True. Perhaps I should make separate
>>   wide_int operator !()
>> and
>>   bool operator bool()
>
> Why not just a comparison?  It seems clearer IMO.

Ok, will do. Guess I won't leave a trace in histor^W wide-int.h this time.

-Y