On Fri, Nov 14, 2014 at 12:40:23AM +0100, Jan Hubicka wrote: > 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
maybe tail recurse optimizer is smart enough to see it can use a jump, and then it might not be too bad. > 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? I believe so, but I haven't actually proved it to myself (I also suggested going this route). I'll try and look in this in more depth, but so many things to do and a bunch of my time the next few days will be at family events. Trev > > 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