Hello,
in general I like this addition - I was not aware that sreal has no support for
negative values.
This would be serious maintainance burden if sreals was used for profile
updating - it is very
easy to get negative temporaries while dong the updates.
> gcc/ChangeLog:
>
> 2014-11-13 Martin Liska <mli...@suse.cz>
>
> * predict.c (propagate_freq): More elegant sreal API is used.
> (estimate_bb_frequencies): New static constants defined by sreal
> replace precomputed ones.
> * sreal.c (sreal::normalize): New function.
> (sreal::to_int): Likewise.
> (sreal::operator+): Likewise.
> (sreal::operator-): Likewise.
> * sreal.h: Definition of new functions added.
> diff --git a/gcc/sreal.c b/gcc/sreal.c
> index 3f5456a..89b9c4d 100644
> --- a/gcc/sreal.c
> +++ b/gcc/sreal.c
> @@ -1,4 +1,4 @@
> -/* Simple data type for positive real numbers for the GNU compiler.
> +/* Simple data type for real numbers for the GNU compiler.
> Copyright (C) 2002-2014 Free Software Foundation, Inc.
>
> This file is part of GCC.
> @@ -17,7 +17,7 @@ You should have received a copy of the GNU General Public
> License
> along with GCC; see the file COPYING3. If not see
> <http://www.gnu.org/licenses/>. */
>
> -/* This library supports positive real numbers and 0;
> +/* This library supports real numbers;
> inf and nan are NOT supported.
> It is written to be simple and fast.
>
> @@ -102,6 +102,7 @@ sreal::normalize ()
> {
> if (m_sig == 0)
> {
> + m_negative = 0;
> m_exp = -SREAL_MAX_EXP;
> }
> else if (m_sig < SREAL_MIN_SIG)
> @@ -153,15 +154,17 @@ sreal::normalize ()
> int64_t
> sreal::to_int () const
> {
> + int64_t sign = m_negative ? -1 : 1;
> +
> if (m_exp <= -SREAL_BITS)
> return 0;
> if (m_exp >= SREAL_PART_BITS)
> - return INTTYPE_MAXIMUM (int64_t);
> + return sign * INTTYPE_MAXIMUM (int64_t);
> if (m_exp > 0)
> - return m_sig << m_exp;
> + return sign * (m_sig << m_exp);
> if (m_exp < 0)
> - return m_sig >> -m_exp;
> - return m_sig;
> + return sign * (m_sig >> -m_exp);
> + return sign * m_sig;
> }
>
> /* Return *this + other. */
> @@ -169,9 +172,19 @@ sreal::to_int () const
> sreal
> sreal::operator+ (const sreal &other) const
> {
> + const sreal *a_p = this, *b_p = &other, *bb;
> +
> + if (m_negative && !other.m_negative)
> + return other + *a_p;
> +
> + if (!m_negative && other.m_negative)
> + return *a_p - -other;
> +
> + gcc_assert (m_negative == other.m_negative);
I wonder what kind of code this winds into - you need recursive inlining to fix
this and it won't be clear to inliner that the recursion depth is 1. Perhaps
having an inline private function for signedless + and - and implementing real
operators by this would factor the code better.
I wonder if there is not a faster implementation given that this code is on
way to internal loops of inliner. If significand was signed, some of these
operations
would just work, right?
> diff --git a/gcc/sreal.h b/gcc/sreal.h
> index 461e28b..bfed3c7 100644
> --- a/gcc/sreal.h
> +++ b/gcc/sreal.h
> @@ -1,4 +1,4 @@
> -/* Definitions for simple data type for positive real numbers.
> +/* Definitions for simple data type for real numbers.
> Copyright (C) 2002-2014 Free Software Foundation, Inc.
>
> This file is part of GCC.
> @@ -25,7 +25,7 @@ along with GCC; see the file COPYING3. If not see
>
> #define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 1))
> #define SREAL_MAX_SIG (((uint64_t) 1 << SREAL_PART_BITS) - 1)
> -#define SREAL_MAX_EXP (INT_MAX / 4)
> +#define SREAL_MAX_EXP (INT_MAX / 8)
>
> #define SREAL_BITS SREAL_PART_BITS
>
> @@ -34,10 +34,21 @@ class sreal
> {
> public:
> /* Construct an uninitialized sreal. */
> - sreal () : m_sig (-1), m_exp (-1) {}
> + sreal () : m_sig (-1), m_exp (-1), m_negative (0) {}
>
> /* Construct a sreal. */
> - sreal (uint64_t sig, int exp) : m_sig (sig), m_exp (exp) { normalize (); }
> + sreal (int64_t sig, int exp = 0) : m_exp (exp)
> + {
> + m_negative = sig < 0;
> +
> + // TODO: speed up
I think we do not need to care about this; expression folding should
> + if (sig < 0)
> + sig = -sig;
> +
> + m_sig = (uint64_t) sig;
> +
> + normalize ();
> + }
We probably want sreal conversion from both signed and unsigned ints?
For debugging output it would be very useful to also have conversion to double.
The dump method is bit unhandy to use.
>
> void dump (FILE *) const;
> int64_t to_int () const;
> @@ -49,13 +60,58 @@ public:
>
> bool operator< (const sreal &other) const
> {
> - return m_exp < other.m_exp
> + if (m_negative != other.m_negative)
> + return m_negative > other.m_negative;
I guess negative zero is smaller than positive, but I do not see why that would
be problem
Again, I would preffer C++ person to glance over this.
Honza
