diff mbox

[8/9] Negative numbers added for sreal class.

Message ID 5485D76B.6080902@suse.cz
State New
Headers show

Commit Message

Martin Liška Dec. 8, 2014, 4:52 p.m. UTC 
On 11/28/2014 10:32 AM, Richard Biener wrote:
> On Thu, Nov 27, 2014 at 6:08 PM, Martin Liška <mliska@suse.cz> wrote:
>> On 11/21/2014 04:21 PM, Martin Liška wrote:
>>>
>>> On 11/21/2014 04:02 PM, Richard Biener wrote:
>>>>
>>>> On Fri, Nov 21, 2014 at 3:39 PM, Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>>> Hello.
>>>>>
>>>>> Ok, this is simplified, one can use sreal a = 12345 and it works ;)
>>>>>
>>>>>> that's a  new API, right?  There is no max () and I think that using
>>>>>> LONG_MIN here is asking for trouble (host dependence).  The
>>>>>> comment in the file says the max should be
>>>>>> sreal (SREAL_MAX_SIG, SREAL_MAX_EXP) and the min
>>>>>> sreal (-SREAL_MAX_SIG, SREAL_MAX_EXP)?
>>>>>>
>>>>>
>>>>> Sure, sreal can store much bigger(smaller) numbers :)
>>>>>
>>>>>> Where do you need sreal::to_double?  The host shouldn't perform
>>>>>> double calculations so it can be only for dumping?  In which case
>>>>>> the user should have used sreal::dump (), maybe with extra
>>>>>> arguments.
>>>>>>
>>>>>
>>>>> That new function was request from Honza, only for debugging purpose.
>>>>> I agree that dump should this kind of job.
>>>>>
>>>>> If no other problem, I will run tests once more and commit it.
>>>>> Thanks,
>>>>> Martin
>>>>
>>>>
>>>> -#define SREAL_MAX_EXP (INT_MAX / 4)
>>>> +#define SREAL_MAX_EXP (INT_MAX / 8)
>>>>
>>>> this change doesn't look necessary anymore?
>>>>
>>>> Btw, it's also odd that...
>>>>
>>>>    #define SREAL_PART_BITS 32
>>>> ...
>>>>    #define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 1))
>>>>    #define SREAL_MAX_SIG (((uint64_t) 1 << SREAL_PART_BITS) - 1)
>>>>
>>>> thus all m_sig values fit in 32bits but we still use a uint64_t m_sig ...
>>>> (the implementation uses 64bit for internal computations, but still
>>>> the storage is wasteful?)
>>>>
>>>> Of course the way normalize() works requires that storage to be
>>>> 64bits to store unnormalized values.
>>>>
>>>> I'd say ok with the SREAL_MAX_EXP change reverted.
>>>>
>>>
>>> Hi.
>>>
>>> You are right, this change was done because I used one bit for m_negative
>>> (bitfield), not needed any more.
>>>
>>> Final version attached.
>>>
>>> Thank you,
>>> Martin
>>>
>>>> Thanks,
>>>> Richard.
>>>>
>>>>
>>>>>
>>>>>> Otherwise looks good to me and sorry for not noticing the above
>>>>>> earlier.
>>>>>>
>>>>>> Thanks,
>>>>>> Richard.
>>>>>>
>>>>>>> Thanks,
>>>>>>> Martin
>>>>>>>
>>>>>>>
>>>>>>>>>      };
>>>>>>>>>
>>>>>>>>>      extern void debug (sreal &ref);
>>>>>>>>> @@ -76,12 +133,12 @@ inline sreal &operator+= (sreal &a, const sreal
>>>>>>>>> &b)
>>>>>>>>>
>>>>>>>>>      inline sreal &operator-= (sreal &a, const sreal &b)
>>>>>>>>>      {
>>>>>>>>> -return a = a - b;
>>>>>>>>> +  return a = a - b;
>>>>>>>>>      }
>>>>>>>>>
>>>>>>>>>      inline sreal &operator/= (sreal &a, const sreal &b)
>>>>>>>>>      {
>>>>>>>>> -return a = a / b;
>>>>>>>>> +  return a = a / b;
>>>>>>>>>      }
>>>>>>>>>
>>>>>>>>>      inline sreal &operator*= (sreal &a, const sreal &b)
>>>>>>>>> --
>>>>>>>>> 2.1.2
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>>
>> Hello.
>>
>> After IRC discussions, I decided to give sreal another refactoring where I
>> use int64_t for m_sig.
>>
>> This approach looks much easier and straightforward. I would like to
>> ask folk for comments?
>
> I think you want to exclude LONG_MIN (how do you know that LONG_MIN
> is min(int64_t)?) as a valid value for m_sig - after all SREAL_MIN_SIG
> makes it symmetric.  Or simply use abs_hwi at

Hello.

I decided to use abs_hwi.

>
> +  int64_t s = m_sig < 0 ? -1 : 1;
> +  uint64_t sig = m_sig == LONG_MIN ? LONG_MAX : std::abs (m_sig);
>
> -#define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 1))
> -#define SREAL_MAX_SIG (((uint64_t) 1 << SREAL_PART_BITS) - 1)
> +#define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 2))
> +#define SREAL_MAX_SIG (((uint64_t) 1 << (SREAL_PART_BITS - 1)) - 1)
>
> shouldn't this also be -2 in the last line?

It's correct, in the first line, I s/'SREAL_PART_BITS - 1'/'SREAL_PART_BITS - 2' and
second one is also decremented: s/'SREAL_PART_BITS'/'REAL_PART_BITS - 1'.

>
> That is, you effectively use the MSB as a sign bit?

Yes. It uses signed integer with MSB as a sign bit.

>
> Btw, a further change would be to make m_sig 'signed int', matching
> the "real" storage requirements according to SREAL_PART_BITS.
> This would of course still require temporaries used for computation
> to be 64bits and it would require normalization to work on the
> temporaries.  But then we'd get down to 8 bytes storage ...

Agree, we can shrink the size for future.

I've been running tests, I hope this implementation is much nicer than
having bool m_negative. What do you think about trunk acceptation?

Thanks,
Martin

>
> Richard.
>
>
>> I am able to run profiled bootstrap on x86_64-linux-pc and ppc64-linux-pc
>> and new regression is introduced.
>>
>> Thanks,
>> Martin
>>
>>

Comments

Richard Biener Dec. 9, 2014, 2:15 p.m. UTC | #1 
On Mon, Dec 8, 2014 at 5:52 PM, Martin Liška <mliska@suse.cz> wrote:
> On 11/28/2014 10:32 AM, Richard Biener wrote:
>>
>> On Thu, Nov 27, 2014 at 6:08 PM, Martin Liška <mliska@suse.cz> wrote:
>>>
>>> On 11/21/2014 04:21 PM, Martin Liška wrote:
>>>>
>>>>
>>>> On 11/21/2014 04:02 PM, Richard Biener wrote:
>>>>>
>>>>>
>>>>> On Fri, Nov 21, 2014 at 3:39 PM, Martin Liška <mliska@suse.cz> wrote:
>>>>>
>>>>>> Hello.
>>>>>>
>>>>>> Ok, this is simplified, one can use sreal a = 12345 and it works ;)
>>>>>>
>>>>>>> that's a  new API, right?  There is no max () and I think that using
>>>>>>> LONG_MIN here is asking for trouble (host dependence).  The
>>>>>>> comment in the file says the max should be
>>>>>>> sreal (SREAL_MAX_SIG, SREAL_MAX_EXP) and the min
>>>>>>> sreal (-SREAL_MAX_SIG, SREAL_MAX_EXP)?
>>>>>>>
>>>>>>
>>>>>> Sure, sreal can store much bigger(smaller) numbers :)
>>>>>>
>>>>>>> Where do you need sreal::to_double?  The host shouldn't perform
>>>>>>> double calculations so it can be only for dumping?  In which case
>>>>>>> the user should have used sreal::dump (), maybe with extra
>>>>>>> arguments.
>>>>>>>
>>>>>>
>>>>>> That new function was request from Honza, only for debugging purpose.
>>>>>> I agree that dump should this kind of job.
>>>>>>
>>>>>> If no other problem, I will run tests once more and commit it.
>>>>>> Thanks,
>>>>>> Martin
>>>>>
>>>>>
>>>>>
>>>>> -#define SREAL_MAX_EXP (INT_MAX / 4)
>>>>> +#define SREAL_MAX_EXP (INT_MAX / 8)
>>>>>
>>>>> this change doesn't look necessary anymore?
>>>>>
>>>>> Btw, it's also odd that...
>>>>>
>>>>>    #define SREAL_PART_BITS 32
>>>>> ...
>>>>>    #define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 1))
>>>>>    #define SREAL_MAX_SIG (((uint64_t) 1 << SREAL_PART_BITS) - 1)
>>>>>
>>>>> thus all m_sig values fit in 32bits but we still use a uint64_t m_sig
>>>>> ...
>>>>> (the implementation uses 64bit for internal computations, but still
>>>>> the storage is wasteful?)
>>>>>
>>>>> Of course the way normalize() works requires that storage to be
>>>>> 64bits to store unnormalized values.
>>>>>
>>>>> I'd say ok with the SREAL_MAX_EXP change reverted.
>>>>>
>>>>
>>>> Hi.
>>>>
>>>> You are right, this change was done because I used one bit for
>>>> m_negative
>>>> (bitfield), not needed any more.
>>>>
>>>> Final version attached.
>>>>
>>>> Thank you,
>>>> Martin
>>>>
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>>>
>>>>>>
>>>>>>> Otherwise looks good to me and sorry for not noticing the above
>>>>>>> earlier.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Richard.
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Martin
>>>>>>>>
>>>>>>>>
>>>>>>>>>>      };
>>>>>>>>>>
>>>>>>>>>>      extern void debug (sreal &ref);
>>>>>>>>>> @@ -76,12 +133,12 @@ inline sreal &operator+= (sreal &a, const
>>>>>>>>>> sreal
>>>>>>>>>> &b)
>>>>>>>>>>
>>>>>>>>>>      inline sreal &operator-= (sreal &a, const sreal &b)
>>>>>>>>>>      {
>>>>>>>>>> -return a = a - b;
>>>>>>>>>> +  return a = a - b;
>>>>>>>>>>      }
>>>>>>>>>>
>>>>>>>>>>      inline sreal &operator/= (sreal &a, const sreal &b)
>>>>>>>>>>      {
>>>>>>>>>> -return a = a / b;
>>>>>>>>>> +  return a = a / b;
>>>>>>>>>>      }
>>>>>>>>>>
>>>>>>>>>>      inline sreal &operator*= (sreal &a, const sreal &b)
>>>>>>>>>> --
>>>>>>>>>> 2.1.2
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>
>>> Hello.
>>>
>>> After IRC discussions, I decided to give sreal another refactoring where
>>> I
>>> use int64_t for m_sig.
>>>
>>> This approach looks much easier and straightforward. I would like to
>>> ask folk for comments?
>>
>>
>> I think you want to exclude LONG_MIN (how do you know that LONG_MIN
>> is min(int64_t)?) as a valid value for m_sig - after all SREAL_MIN_SIG
>> makes it symmetric.  Or simply use abs_hwi at
>
>
> Hello.
>
> I decided to use abs_hwi.

That will ICE if you do

  sreal x (- __LONG_MAX__ - 1);

maybe that's the only case though.

 sreal::normalize ()
 {
+  int64_t s = m_sig < 0 ? -1 : 1;
+  HOST_WIDE_INT sig = abs_hwi (m_sig);
+
   if (m_sig == 0)
...
     }
+
+  m_sig = s * sig;
 }

it's a bit awkward to strip the sign and then put it back on this way.  Also
now using a signed 'sig' where it was unsigned before.  And keeping
the first test using m_sig instead of sig.

I'd simply have used 'unsigned HOST_WIDE_INT sig = absu_hwi (m_sig);'
instead.

The rest of the patch is ok with the above change.

Thanks,
Richard.


>>
>> +  int64_t s = m_sig < 0 ? -1 : 1;
>> +  uint64_t sig = m_sig == LONG_MIN ? LONG_MAX : std::abs (m_sig);
>>
>> -#define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 1))
>> -#define SREAL_MAX_SIG (((uint64_t) 1 << SREAL_PART_BITS) - 1)
>> +#define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 2))
>> +#define SREAL_MAX_SIG (((uint64_t) 1 << (SREAL_PART_BITS - 1)) - 1)
>>
>> shouldn't this also be -2 in the last line?
>
>
> It's correct, in the first line, I s/'SREAL_PART_BITS - 1'/'SREAL_PART_BITS
> - 2' and
> second one is also decremented: s/'SREAL_PART_BITS'/'REAL_PART_BITS - 1'.
>
>>
>> That is, you effectively use the MSB as a sign bit?
>
>
> Yes. It uses signed integer with MSB as a sign bit.
>
>>
>> Btw, a further change would be to make m_sig 'signed int', matching
>> the "real" storage requirements according to SREAL_PART_BITS.
>> This would of course still require temporaries used for computation
>> to be 64bits and it would require normalization to work on the
>> temporaries.  But then we'd get down to 8 bytes storage ...
>
>
> Agree, we can shrink the size for future.
>
> I've been running tests, I hope this implementation is much nicer than
> having bool m_negative. What do you think about trunk acceptation?
>
> Thanks,
> Martin
>
>
>>
>> Richard.
>>
>>
>>> I am able to run profiled bootstrap on x86_64-linux-pc and ppc64-linux-pc
>>> and new regression is introduced.
>>>
>>> Thanks,
>>> Martin
>>>
>>>
>
diff mbox

Patch

From c275c48eb5f0f41f546bb7866d70c1fe066d6d62 Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Wed, 26 Nov 2014 15:46:42 +0100
Subject: [PATCH] New sreal implementation which uses int64_t as m_sig.

gcc/ChangeLog:

2014-12-08  Martin Liska  <mliska@suse.cz>

	* sreal.c (sreal::shift_right): New implementation
	for int64_t as m_sig.
	(sreal::normalize): Likewise.
	(sreal::to_int): Likewise.
	(sreal::operator+): Likewise.
	(sreal::operator-): Likewise.
	(sreal::operator*): Likewise.
	(sreal::operator/): Likewise.
	(sreal::signedless_minus): Removed.
	(sreal::signedless_plus): Removed.
	(sreal::debug): const keyword is added.
	* sreal.h (sreal::operator<): New implementation
	for int64_t as m_sig.
	* ipa-inline.c (recursive_inlining): LONG_MIN is replaced
	with sreal::min ().
---
 gcc/ipa-inline.c |   2 +-
 gcc/sreal.c      | 129 +++++++++++++++++--------------------------------------
 gcc/sreal.h      |  56 ++++++++++--------------
 3 files changed, 62 insertions(+), 125 deletions(-)

diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 26335ec..a5044fd 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -1311,7 +1311,7 @@  recursive_inlining (struct cgraph_edge *edge,
 		    vec<cgraph_edge *> *new_edges)
 {
   int limit = PARAM_VALUE (PARAM_MAX_INLINE_INSNS_RECURSIVE_AUTO);
-  edge_heap_t heap (LONG_MIN);
+  edge_heap_t heap (sreal::min ());
   struct cgraph_node *node;
   struct cgraph_edge *e;
   struct cgraph_node *master_clone = NULL, *next;
diff --git a/gcc/sreal.c b/gcc/sreal.c
index 2b5e3ae..9ff0109 100644
--- a/gcc/sreal.c
+++ b/gcc/sreal.c
@@ -61,13 +61,13 @@  sreal::dump (FILE *file) const
 }
 
 DEBUG_FUNCTION void
-debug (sreal &ref)
+debug (const sreal &ref)
 {
   ref.dump (stderr);
 }
 
 DEBUG_FUNCTION void
-debug (sreal *ptr)
+debug (const sreal *ptr)
 {
   if (ptr)
     debug (*ptr);
@@ -91,7 +91,7 @@  sreal::shift_right (int s)
 
   m_exp += s;
 
-  m_sig += (uint64_t) 1 << (s - 1);
+  m_sig += (int64_t) 1 << (s - 1);
   m_sig >>= s;
 }
 
@@ -100,43 +100,45 @@  sreal::shift_right (int s)
 void
 sreal::normalize ()
 {
+  int64_t s = m_sig < 0 ? -1 : 1;
+  HOST_WIDE_INT sig = abs_hwi (m_sig);
+
   if (m_sig == 0)
     {
-      m_negative = 0;
       m_exp = -SREAL_MAX_EXP;
     }
-  else if (m_sig < SREAL_MIN_SIG)
+  else if (sig < SREAL_MIN_SIG)
     {
       do
 	{
-	  m_sig <<= 1;
+	  sig <<= 1;
 	  m_exp--;
 	}
-      while (m_sig < SREAL_MIN_SIG);
+      while (sig < SREAL_MIN_SIG);
 
       /* Check underflow.  */
       if (m_exp < -SREAL_MAX_EXP)
 	{
 	  m_exp = -SREAL_MAX_EXP;
-	  m_sig = 0;
+	  sig = 0;
 	}
     }
-  else if (m_sig > SREAL_MAX_SIG)
+  else if (sig > SREAL_MAX_SIG)
     {
       int last_bit;
       do
 	{
-	  last_bit = m_sig & 1;
-	  m_sig >>= 1;
+	  last_bit = sig & 1;
+	  sig >>= 1;
 	  m_exp++;
 	}
-      while (m_sig > SREAL_MAX_SIG);
+      while (sig > SREAL_MAX_SIG);
 
       /* Round the number.  */
-      m_sig += last_bit;
-      if (m_sig > SREAL_MAX_SIG)
+      sig += last_bit;
+      if (sig > SREAL_MAX_SIG)
 	{
-	  m_sig >>= 1;
+	  sig >>= 1;
 	  m_exp++;
 	}
 
@@ -144,9 +146,11 @@  sreal::normalize ()
       if (m_exp > SREAL_MAX_EXP)
 	{
 	  m_exp = SREAL_MAX_EXP;
-	  m_sig = SREAL_MAX_SIG;
+	  sig = SREAL_MAX_SIG;
 	}
     }
+
+  m_sig = s * sig;
 }
 
 /* Return integer value of *this.  */
@@ -154,17 +158,17 @@  sreal::normalize ()
 int64_t
 sreal::to_int () const
 {
-  int64_t sign = m_negative ? -1 : 1;
+  int64_t sign = m_sig < 0 ? -1 : 1;
 
   if (m_exp <= -SREAL_BITS)
     return 0;
   if (m_exp >= SREAL_PART_BITS)
     return sign * INTTYPE_MAXIMUM (int64_t);
   if (m_exp > 0)
-    return sign * (m_sig << m_exp);
+    return m_sig << m_exp;
   if (m_exp < 0)
-    return sign * (m_sig >> -m_exp);
-  return sign * m_sig;
+    return m_sig >> -m_exp;
+  return m_sig;
 }
 
 /* Return *this + other.  */
@@ -172,36 +176,10 @@  sreal::to_int () const
 sreal
 sreal::operator+ (const sreal &other) const
 {
-  const sreal *a_p = this, *b_p = &other;
-
-  if (a_p->m_negative && !b_p->m_negative)
-    std::swap (a_p, b_p);
-
-  /* a + -b => a - b.  */
-  if (!a_p->m_negative && b_p->m_negative)
-    {
-      sreal tmp = -(*b_p);
-      if (*a_p < tmp)
-	return signedless_minus (tmp, *a_p, true);
-      else
-	return signedless_minus (*a_p, tmp, false);
-    }
-
-  gcc_checking_assert (a_p->m_negative == b_p->m_negative);
-
-  sreal r = signedless_plus (*a_p, *b_p, a_p->m_negative);
-
-  return r;
-}
-
-sreal
-sreal::signedless_plus (const sreal &a, const sreal &b, bool negative)
-{
-  const sreal *bb;
-  sreal r, tmp;
   int dexp;
-  const sreal *a_p = &a;
-  const sreal *b_p = &b;
+  sreal tmp, r;
+
+  const sreal *a_p = this, *b_p = &other, *bb;
 
   if (a_p->m_exp < b_p->m_exp)
     std::swap (a_p, b_p);
@@ -211,7 +189,6 @@  sreal::signedless_plus (const sreal &a, const sreal &b, bool negative)
   if (dexp > SREAL_BITS)
     {
       r.m_sig = a_p->m_sig;
-      r.m_negative = negative;
       return r;
     }
 
@@ -226,56 +203,32 @@  sreal::signedless_plus (const sreal &a, const sreal &b, bool negative)
 
   r.m_sig = a_p->m_sig + bb->m_sig;
   r.normalize ();
-
-  r.m_negative = negative;
   return r;
 }
 
+
 /* Return *this - other.  */
 
 sreal
 sreal::operator- (const sreal &other) const
 {
-  /* -a - b => -a + (-b).  */
-  if (m_negative && !other.m_negative)
-    return signedless_plus (*this, -other, true);
-
-  /* a - (-b) => a + b.  */
-  if (!m_negative && other.m_negative)
-    return signedless_plus (*this, -other, false);
-
-  gcc_checking_assert (m_negative == other.m_negative);
-
-  /* We want to substract a smaller number from bigger
-    for nonegative numbers.  */
-  if (!m_negative && *this < other)
-    return signedless_minus (other, *this, true);
-
-  /* Example: -2 - (-3) => 3 - 2 */
-  if (m_negative && *this > other)
-    return signedless_minus (-other, -(*this), false);
-
-  sreal r = signedless_minus (*this, other, m_negative);
-
-  return r;
-}
-
-sreal
-sreal::signedless_minus (const sreal &a, const sreal &b, bool negative)
-{
   int dexp;
   sreal tmp, r;
   const sreal *bb;
-  const sreal *a_p = &a;
-  const sreal *b_p = &b;
+  const sreal *a_p = this, *b_p = &other;
 
-  dexp = a_p->m_exp - b_p->m_exp;
+  int64_t sign = 1;
+  if (a_p->m_exp < b_p->m_exp)
+    {
+      sign = -1;
+      std::swap (a_p, b_p);
+    }
 
+  dexp = a_p->m_exp - b_p->m_exp;
   r.m_exp = a_p->m_exp;
   if (dexp > SREAL_BITS)
     {
-      r.m_sig = a_p->m_sig;
-      r.m_negative = negative;
+      r.m_sig = sign * a_p->m_sig;
       return r;
     }
   if (dexp == 0)
@@ -287,10 +240,8 @@  sreal::signedless_minus (const sreal &a, const sreal &b, bool negative)
       bb = &tmp;
     }
 
-  r.m_sig = a_p->m_sig - bb->m_sig;
+  r.m_sig = sign * (a_p->m_sig - bb->m_sig);
   r.normalize ();
-
-  r.m_negative = negative;
   return r;
 }
 
@@ -300,7 +251,7 @@  sreal
 sreal::operator* (const sreal &other) const
 {
   sreal r;
-  if (m_sig < SREAL_MIN_SIG || other.m_sig < SREAL_MIN_SIG)
+  if (std::abs (m_sig) < SREAL_MIN_SIG || std::abs (other.m_sig) < SREAL_MIN_SIG)
     {
       r.m_sig = 0;
       r.m_exp = -SREAL_MAX_EXP;
@@ -312,7 +263,6 @@  sreal::operator* (const sreal &other) const
       r.normalize ();
     }
 
-  r.m_negative = m_negative ^ other.m_negative;
   return r;
 }
 
@@ -325,7 +275,6 @@  sreal::operator/ (const sreal &other) const
   sreal r;
   r.m_sig = (m_sig << SREAL_PART_BITS) / other.m_sig;
   r.m_exp = m_exp - other.m_exp - SREAL_PART_BITS;
-  r.m_negative = m_negative ^ other.m_negative;
   r.normalize ();
   return r;
 }
diff --git a/gcc/sreal.h b/gcc/sreal.h
index 3938c6e..730f49c 100644
--- a/gcc/sreal.h
+++ b/gcc/sreal.h
@@ -25,8 +25,8 @@  along with GCC; see the file COPYING3.  If not see
 
 #define UINT64_BITS	64
 
-#define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 1))
-#define SREAL_MAX_SIG (((uint64_t) 1 << SREAL_PART_BITS) - 1)
+#define SREAL_MIN_SIG ((int64_t) 1 << (SREAL_PART_BITS - 2))
+#define SREAL_MAX_SIG (((int64_t) 1 << (SREAL_PART_BITS - 1)) - 1)
 #define SREAL_MAX_EXP (INT_MAX / 4)
 
 #define SREAL_BITS SREAL_PART_BITS
@@ -36,18 +36,11 @@  class sreal
 {
 public:
   /* Construct an uninitialized sreal.  */
-  sreal () : m_sig (-1), m_exp (-1), m_negative (0) {}
+  sreal () : m_sig (-1), m_exp (-1) {}
 
   /* Construct a sreal.  */
-  sreal (int64_t sig, int exp = 0) : m_exp (exp)
+  sreal (int64_t sig, int exp = 0) : m_sig (sig), m_exp (exp)
   {
-    m_negative = sig < 0;
-
-    if (sig < 0)
-      sig = -sig;
-
-    m_sig = (uint64_t) sig;
-
     normalize ();
   }
 
@@ -60,33 +53,30 @@  public:
 
   bool operator< (const sreal &other) const
   {
-    /* We negate result in case of negative numbers and
-       it would return true for equal negative numbers.  */
-    if (*this == other)
-      return false;
-
-    if (m_negative != other.m_negative)
-      return m_negative > other.m_negative;
-
-    bool r = m_exp < other.m_exp
-      || (m_exp == other.m_exp && m_sig < other.m_sig);
-
-    return m_negative ? !r : r;
+    if (m_exp == other.m_exp)
+      return m_sig < other.m_sig;
+    else
+    {
+      bool negative = m_sig < 0;
+      bool other_negative = other.m_sig < 0;
+
+      if (negative != other_negative)
+        return negative > other_negative;
+
+      bool r = m_exp < other.m_exp;
+      return negative ? !r : r;
+    }
   }
 
   bool operator== (const sreal &other) const
   {
-    return m_exp == other.m_exp && m_sig == other.m_sig
-		    && m_negative == other.m_negative;
+    return m_exp == other.m_exp && m_sig == other.m_sig;
   }
 
   sreal operator- () const
   {
-    if (m_sig == 0)
-      return *this;
-
     sreal tmp = *this;
-    tmp.m_negative = !tmp.m_negative;
+    tmp.m_sig *= -1;
 
     return tmp;
   }
@@ -125,17 +115,15 @@  public:
 private:
   void normalize ();
   void shift_right (int amount);
-
   static sreal signedless_plus (const sreal &a, const sreal &b, bool negative);
   static sreal signedless_minus (const sreal &a, const sreal &b, bool negative);
 
-  uint64_t m_sig;			/* Significant.  */
+  int64_t m_sig;			/* Significant.  */
   signed int m_exp;			/* Exponent.  */
-  bool m_negative;			/* Negative sign.  */
 };
 
-extern void debug (sreal &ref);
-extern void debug (sreal *ptr);
+extern void debug (const sreal &ref);
+extern void debug (const sreal *ptr);
 
 inline sreal &operator+= (sreal &a, const sreal &b)
 {
-- 
2.1.2