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

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

[Yuri Gribov]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0003-Added-bool-conversion-for-wide_ints.patch --]
[-- Type: application/octet-stream, Size: 2889 bytes --]

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.
---
 gcc/wide-int.cc | 10 ++++++++++
 gcc/wide-int.h  | 17 +++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/gcc/wide-int.cc b/gcc/wide-int.cc
index dab4c19..f1be89b 100644
--- a/gcc/wide-int.cc
+++ b/gcc/wide-int.cc
@@ -433,6 +433,16 @@ top_bit_of (const HOST_WIDE_INT *a, unsigned int len, unsigned int prec)
  * unsigned and C++ has no such operators.
  */
 
+/* Return true if OP == 0.  */
+bool
+wi::zero_p_large (const HOST_WIDE_INT *op, unsigned int len)
+{
+  for (unsigned i = 0; i < len; ++i)
+    if (op[i])
+      return false;
+  return true;
+}
+
 /* Return true if OP0 == OP1.  */
 bool
 wi::eq_p_large (const HOST_WIDE_INT *op0, unsigned int op0len,
diff --git a/gcc/wide-int.h b/gcc/wide-int.h
index 2115b61..af63ffe 100644
--- a/gcc/wide-int.h
+++ b/gcc/wide-int.h
@@ -462,6 +462,7 @@ namespace wi
   UNARY_PREDICATE fits_shwi_p (const T &);
   UNARY_PREDICATE fits_uhwi_p (const T &);
   UNARY_PREDICATE neg_p (const T &, signop = SIGNED);
+  UNARY_PREDICATE zero_p (const T &);
 
   template <typename T>
   HOST_WIDE_INT sign_mask (const T &);
@@ -675,6 +676,9 @@ public:
   template <typename T>
   generic_wide_int &operator = (const T &);
 
+#define UNARY_PREDICATE(OP, F) \
+  bool OP () const { return wi::F (*this); }
+
 #define BINARY_PREDICATE(OP, F) \
   template <typename T> \
   bool OP (const T &c) const { return wi::F (*this, c); }
@@ -699,6 +703,7 @@ public:
 #define INCDEC_OPERATOR(OP, DELTA) \
   generic_wide_int &OP () { *this += DELTA; return *this; }
 
+  UNARY_PREDICATE (operator !, zero_p)
   UNARY_OPERATOR (operator ~, bit_not)
   UNARY_OPERATOR (operator -, neg)
   BINARY_PREDICATE (operator ==, eq_p)
@@ -1605,6 +1610,7 @@ decompose (HOST_WIDE_INT *scratch, unsigned int precision,
    we generally want those to be removed by SRA.)  */
 namespace wi
 {
+  bool zero_p_large (const HOST_WIDE_INT *, unsigned int);
   bool eq_p_large (const HOST_WIDE_INT *, unsigned int,
 		   const HOST_WIDE_INT *, unsigned int, unsigned int);
   bool lts_p_large (const HOST_WIDE_INT *, unsigned int, unsigned int,
@@ -1729,6 +1735,17 @@ wi::neg_p (const T &x, signop sgn)
   return xi.sign_mask () < 0;
 }
 
+/* Return true if X is zero.  */
+template <typename T>
+inline bool
+wi::zero_p (const T &x)
+{
+  WIDE_INT_REF_FOR (T) xi (x);
+  if (__builtin_expect (xi.len == 1, true))
+    return !xi.val[0];
+  return zero_p_large (xi.val, xi.len);
+}
+
 /* Return -1 if the top bit of X is set and 0 if the top bit is clear.  */
 template <typename T>
 inline HOST_WIDE_INT
-- 
2.7.4

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

[Richard Sandiford]

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

> ---
>  gcc/wide-int.cc | 10 ++++++++++
>  gcc/wide-int.h  | 17 +++++++++++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/gcc/wide-int.cc b/gcc/wide-int.cc
> index dab4c19..f1be89b 100644
> --- a/gcc/wide-int.cc
> +++ b/gcc/wide-int.cc
> @@ -433,6 +433,16 @@ top_bit_of (const HOST_WIDE_INT *a, unsigned int len, unsigned int prec)
>   * unsigned and C++ has no such operators.
>   */
>  
> +/* Return true if OP == 0.  */
> +bool
> +wi::zero_p_large (const HOST_WIDE_INT *op, unsigned int len)
> +{
> +  for (unsigned i = 0; i < len; ++i)
> +    if (op[i])
> +      return false;
> +  return true;
> +}
> +
>  /* Return true if OP0 == OP1.  */
>  bool
>  wi::eq_p_large (const HOST_WIDE_INT *op0, unsigned int op0len,
> diff --git a/gcc/wide-int.h b/gcc/wide-int.h
> index 2115b61..af63ffe 100644
> --- a/gcc/wide-int.h
> +++ b/gcc/wide-int.h
> @@ -462,6 +462,7 @@ namespace wi
>    UNARY_PREDICATE fits_shwi_p (const T &);
>    UNARY_PREDICATE fits_uhwi_p (const T &);
>    UNARY_PREDICATE neg_p (const T &, signop = SIGNED);
> +  UNARY_PREDICATE zero_p (const T &);
>  
>    template <typename T>
>    HOST_WIDE_INT sign_mask (const T &);
> @@ -675,6 +676,9 @@ public:
>    template <typename T>
>    generic_wide_int &operator = (const T &);
>  
> +#define UNARY_PREDICATE(OP, F) \
> +  bool OP () const { return wi::F (*this); }
> +
>  #define BINARY_PREDICATE(OP, F) \
>    template <typename T> \
>    bool OP (const T &c) const { return wi::F (*this, c); }
> @@ -699,6 +703,7 @@ public:
>  #define INCDEC_OPERATOR(OP, DELTA) \
>    generic_wide_int &OP () { *this += DELTA; return *this; }
>  
> +  UNARY_PREDICATE (operator !, zero_p)
>    UNARY_OPERATOR (operator ~, bit_not)
>    UNARY_OPERATOR (operator -, neg)
>    BINARY_PREDICATE (operator ==, eq_p)
> @@ -1605,6 +1610,7 @@ decompose (HOST_WIDE_INT *scratch, unsigned int precision,
>     we generally want those to be removed by SRA.)  */
>  namespace wi
>  {
> +  bool zero_p_large (const HOST_WIDE_INT *, unsigned int);
>    bool eq_p_large (const HOST_WIDE_INT *, unsigned int,
>  		   const HOST_WIDE_INT *, unsigned int, unsigned int);
>    bool lts_p_large (const HOST_WIDE_INT *, unsigned int, unsigned int,
> @@ -1729,6 +1735,17 @@ wi::neg_p (const T &x, signop sgn)
>    return xi.sign_mask () < 0;
>  }
>  
> +/* Return true if X is zero.  */
> +template <typename T>
> +inline bool
> +wi::zero_p (const T &x)
> +{
> +  WIDE_INT_REF_FOR (T) xi (x);
> +  if (__builtin_expect (xi.len == 1, true))
> +    return !xi.val[0];
> +  return zero_p_large (xi.val, xi.len);
> +}
> +
>  /* Return -1 if the top bit of X is set and 0 if the top bit is clear.  */
>  template <typename T>
>  inline HOST_WIDE_INT

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

[Yuri Gribov]

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));

-Y

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

[Richard Sandiford]

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).  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.

Thanks,
Richard

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

[Yuri Gribov]

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.

> 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()

-I

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

[Richard Sandiford]

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.

Thanks,
Richard

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

[Yuri Gribov]

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