[DTrace-devel] [PATCH 06/13] Add code for llquantize() aggregation function

[Kris Van Hees]

On Tue, Dec 08, 2020 at 11:01:12AM -0800, Eugene Loh wrote:
> I'll incorporate your feedback, but have a few questions...
> 
> 
> On 12/08/2020 10:08 AM, Kris Van Hees wrote:
> 
>     On Wed, Dec 02, 2020 at 01:54:51PM -0500, eugene.loh at oracle.com wrote:
> 
>         From: Eugene Loh <eugene.loh at oracle.com>
> 
>         diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>         + * The bins are arranged in numerical order, starting with negative underflow,
>         + * ending with positive overflow, and with underflow exactly in the middle.
> 
>     This terminology seems a bit odd to me.  Is it what is used in documentation
>     or in the original code, or is it new?  I would consider the lowest order
>     bin to be underflow (less than the lower limit), the highest order bin to be
>     overflow (greater than the higher limit), and what you refer to as the
>     underflow bin as the zero bin (where zero is 0 +- some delta of course.
> 
> 
> First, the "negative underflow" was supposed to be "negative overflow."Â 
> Anyhow, documentation doesn't discuss llquantize too much;Â  the output is
> supposed to be sufficiently self explanatory and a description likely to be at
> least as confusing as helpful.  The idea was something along the line of how
> one might order fixed-size, floating-point numbers:Â  -Inf, negative numbers,
> Underflow, positive numbers, +Inf.  So, -Inf is overflow (albeit in the
> negative direction) rather than underflow.  E.g., the picture shown here:
> http://www.teach-ict.com/as_as_computing/ocr/H447/F453/3_3_4/floating_point/
> miniweb/images/overflow.gif
> 
> Does that help?  Maybe the problem was my saying "negative underflow" when I
> meant "negative overflow."

Yes, with "negative undeflow" it is more clear.  Thanks.
> 
> 
>         + */
>         +static void
>         +dt_cg_agg_llquantize_bin(dt_irlist_t *dlp, dt_regset_t *drp, int valreg,
>         +                        int32_t factor, int32_t lmag, int32_t hmag,
>         +                        int32_t steps)
>         +{
>         +       /*
>         +        * We say there are "steps" bins per logarithmic range,
>         +        * but steps/factor of them actually overlap with lower ranges.
>         +        */
>         +       int steps_factor = steps / factor;
>         +
>         +       /* the underflow bin is in the middle */
>         +       int underflow_bin = 1 + (hmag - lmag + 1) * (steps - steps_factor);
>         +
>         +       /* initial bucket_max */
>         +       uint64_t bucket_max0 = powl(factor, lmag);
>         +
>         +       /* registers */
>         +       int indreg, magreg, tmpreg, maxreg;
>         +
>         +       /* labels */
>         +       uint_t          L1 = dt_irlist_label(dlp);
>         +       uint_t          L2 = dt_irlist_label(dlp);
>         +       uint_t          L3 = dt_irlist_label(dlp);
>         +       uint_t          L4 = dt_irlist_label(dlp);
>         +       uint_t          Lloop = dt_irlist_label(dlp);
>         +       uint_t          Lbin = dt_irlist_label(dlp);
>         +       uint_t          Lshift = dt_irlist_label(dlp);
>         +       uint_t          Lend = dt_irlist_label(dlp);
>         +
>         +       TRACE_REGSET("            Impl: Begin");
> 
>     This should be some other regset trace tag because Impl: is used for the
>     implementation code for the aggregation data update.  Having it here would be
>     confusing.  Perhaps something like "Bin: Begin" (and end with "Bin: End  ")?
> 
> 
> Is that the indentation you want?  Flush with the left margin?

Ah no, sorry if my feedback was confusing...  I meant replacing the text
portion of the string Impl: Begin with Bin: Befin, so keeping the indent
that was there.

> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel