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 <mli...@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 am able to run profiled bootstrap on x86_64-linux-pc and ppc64-linux-pc
and new regression is introduced.
Thanks,
Martin
>From bff0b4b803271788cd90cfd4032ed6d4e6e95707 Mon Sep 17 00:00:00 2001
From: mliska <mli...@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-11-27 Martin Liska <mli...@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.h (sreal::operator<): New implementation
for int64_t as m_sig.
---
gcc/sreal.c | 129 +++++++++++++++++++-----------------------------------------
gcc/sreal.h | 52 ++++++++++--------------
2 files changed, 61 insertions(+), 120 deletions(-)
diff --git a/gcc/sreal.c b/gcc/sreal.c
index 2b5e3ae..304feb0 100644
--- a/gcc/sreal.c
+++ b/gcc/sreal.c
@@ -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,46 @@ sreal::shift_right (int s)
void
sreal::normalize ()
{
+ int64_t s = m_sig < 0 ? -1 : 1;
+ uint64_t sig = m_sig == LONG_MIN ? LONG_MAX : std::abs (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--;
+ gcc_checking_assert (sig);
}
- 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 +147,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 +159,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 +177,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 +190,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 +204,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 +241,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 +252,10 @@ sreal
sreal::operator* (const sreal &other) const
{
sreal r;
- if (m_sig < SREAL_MIN_SIG || other.m_sig < SREAL_MIN_SIG)
+ uint64_t asig = (uint64_t)std::abs (m_sig);
+ uint64_t other_asig = (uint64_t)std::abs (other.m_sig);
+
+ if (asig < SREAL_MIN_SIG || other_asig < SREAL_MIN_SIG)
{
r.m_sig = 0;
r.m_exp = -SREAL_MAX_EXP;
@@ -312,7 +267,6 @@ sreal::operator* (const sreal &other) const
r.normalize ();
}
- r.m_negative = m_negative ^ other.m_negative;
return r;
}
@@ -325,7 +279,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..8b3c6de 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 ((uint64_t) 1 << (SREAL_PART_BITS - 2))
+#define SREAL_MAX_SIG (((uint64_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,13 +115,11 @@ 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);
--
2.1.2
