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

[Eugene Loh]

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."

>> + */
>> +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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/dtrace-devel/attachments/20201208/e2b2e7df/attachment.html