Re: Fix sreal::shift add to_double conversion

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Dec 4, 2014 at 9:06 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On December 4, 2014 7:00:12 PM CET, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >Hi,
>> >this patch fixes implementation of shift in sreal.h that ICEs on 0
>> >value.
>> >Also the comment is copied from shift_right and does not make sense for
>> >sreal.h.
>> >
>> >The to_double conversion is useful for debug output: Most of inliner
>> >data is
>> >now output as integer that does not make much sense.
>>
>> Can't you use dump for this?
>
> Well, dump will get you somewhat less readable form like 1234*2^3 and also it makes it hard to dump
> multiple values with single printf.
>       if (dump)
>         {
>           sreal num,den;
>           relative_time_benefit (callee_info, edge, edge_time, &num, &den);
>           fprintf (dump_file,
>                    "      %f: guessed profile. frequency %f,"
>                    " benefit %f%%, time w/o inlining %f, time w inlining %f"
>                    " overall growth %i (current) %i (original)\n",
>                    badness.to_double (), (double)edge->frequency / CGRAPH_FREQ_BASE,
>                    (num/den).to_double () * 100,
>                    compute_uninlined_call_time (callee_info, edge).to_double (),
>                    compute_inlined_call_time (edge, edge_time).to_double (),
>                    estimate_growth (callee),
>                    callee_info->growth);
>         }
>
> would look even more ugly if done with dump method.

Hmm, ok.  Please add a comment before to_double that it is only
supposed to be used for debug dumping.

Richard.

> Honza
>>
>> Richard.
>>
>> >Bootstrapped/regtested x86_64-linux, OK?
>> >
>> >     * sreal.h (to_double): Declare.
>> >     (shift): Do not ICE on 0, update outdated comment.
>> >     * sreal.c: Include math.h
>> >     (sreal::to_double): New function.
>> >Index: sreal.h
>> >===================================================================
>> >--- sreal.h  (revision 218286)
>> >+++ sreal.h  (working copy)
>> >@@ -53,6 +53,7 @@
>> >
>> >   void dump (FILE *) const;
>> >   int64_t to_int () const;
>> >+  double to_double () const;
>> >   sreal operator+ (const sreal &other) const;
>> >   sreal operator- (const sreal &other) const;
>> >   sreal operator* (const sreal &other) const;
>> >@@ -93,12 +94,14 @@
>> >
>> >   sreal shift (int s) const
>> >   {
>> >+    /* Zero needs no shifting.  */
>> >+    if (!m_sig)
>> >+      return *this;
>> >     gcc_checking_assert (s <= SREAL_BITS);
>> >     gcc_checking_assert (s >= -SREAL_BITS);
>> >
>> >-    /* Exponent should never be so large because shift_right is used
>> >only by
>> >-     sreal_add and sreal_sub ant thus the number cannot be shifted out
>> >from
>> >-     exponent range.  */
>> >+    /* Overflows/drop to 0 could be handled gracefully, but hopefully
>> >we do not
>> >+       need to do so.  */
>> >     gcc_checking_assert (m_exp + s <= SREAL_MAX_EXP);
>> >     gcc_checking_assert (m_exp + s >= -SREAL_MAX_EXP);
>> >
>> >Index: sreal.c
>> >===================================================================
>> >--- sreal.c  (revision 218286)
>> >+++ sreal.c  (working copy)
>> >@@ -47,6 +47,7 @@
>> >     sig == 0 && exp == -SREAL_MAX_EXP
>> > */
>> >
>> >+#include <math.h>
>> > #include "config.h"
>> > #include "system.h"
>> > #include "coretypes.h"
>> >@@ -167,6 +168,19 @@
>> >   return sign * m_sig;
>> > }
>> >
>> >+/* Return value of *this as double.  */
>> >+
>> >+double
>> >+sreal::to_double () const
>> >+{
>> >+  double val = m_sig;
>> >+  if (m_negative)
>> >+    val = -val;
>> >+  if (m_exp)
>> >+    val *= exp2 (m_exp);
>> >+  return val;
>> >+}
>> >+
>> > /* Return *this + other.  */
>> >
>> > sreal
>>