svn commit: r209604 - head/lib/libc/gmon

[Bruce Evans]

On Wed, 30 Jun 2010, Marcel Moolenaar wrote:

> On Jun 30, 2010, at 2:40 AM, Bruce Evans wrote:
>
>> On Wed, 30 Jun 2010, Marcel Moolenaar wrote:
>>
>>> Log:
>>> On powerpc, calculate s_scale using the non-FP version previously
>>> specific to hp300. Since FreeBSD does not support hp300, hp300 has
>>> been removed from the condition altogether.
>>
>> Also, the style of the condition has been regressed from ifndef to
>> if !defined().
>
> That's on purpose. Either we eliminate the whole conditional or we
> end up adding other platforms to it.

Better to inhibit such mistakes.  It's almost a sign of a mistake that
there is a long list of platforms in an ifdef.

> ...
>> Better still, rewrite the integer method using a C99 type, so that it
>> is as simple as the FP method:
>>
>> 		s_scale = ((uintmax_t)p->kcountsize << SCALE_SHIFT) / o;
>>
>> C99 uintmax_t now guarantees uintmax_t to have >= 64 bits, and practical
>> considerations guarantee p->kcountsize to fit in many fewer than 48 bits
>> even on 64-bit arches, so the shift cannot overflow.
>
> I like this. What about the following (white-space corrupted)
> simplification:

Almost OK.  I orginally wrote it with a conditional operator, but decided
that that might be harder to understand.

> Index: gmon.c
> ===================================================================
> --- gmon.c	(revision 209604)
> +++ gmon.c	(working copy)
> @@ -110,24 +110,9 @@
> 	p->tos[0].link = 0;
>
> 	o = p->highpc - p->lowpc;
> -	if (p->kcountsize < o) {
> -#if !defined(__powerpc__)
> -		s_scale = ((float)p->kcountsize / o ) * SCALE_1_TO_1;
> -#else /* avoid floating point */
> -		int quot = o / p->kcountsize;
> +	s_scale = (p->kcountsize < o) ?
> +	    ((uintmax_t)p->kcountsize << SCALE_1_TO_1) / o : SCALE_1_TO_1;

Shifting by 65536 is a bit much :-).  Multiplication by 65536 might
give the same code as shifting by 16, but I think shifting is clearer.

The kernel uses shifting for the reverse conversion, with `16' hard-coded
in 2 statements and 2 comments, and with too much duplication in the
comments.

>

Remove this blank line too.

> -		if (quot >= 0x10000)
> -			s_scale = 1;
> ...

Bruce