From owner-svn-src-all@FreeBSD.ORG Wed Jun 30 09:40:28 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 41A6C106566B; Wed, 30 Jun 2010 09:40:28 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail05.syd.optusnet.com.au (mail05.syd.optusnet.com.au [211.29.132.186]) by mx1.freebsd.org (Postfix) with ESMTP id BDFF88FC1C; Wed, 30 Jun 2010 09:40:27 +0000 (UTC) Received: from c122-106-145-25.carlnfd1.nsw.optusnet.com.au (c122-106-145-25.carlnfd1.nsw.optusnet.com.au [122.106.145.25]) by mail05.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o5U9eMhA005698 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 30 Jun 2010 19:40:24 +1000 Date: Wed, 30 Jun 2010 19:40:22 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Marcel Moolenaar In-Reply-To: <201006300140.o5U1eQVG097566@svn.freebsd.org> Message-ID: <20100630184517.B51465@delplex.bde.org> References: <201006300140.o5U1eQVG097566@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r209604 - head/lib/libc/gmon X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Jun 2010 09:40:28 -0000 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(). > The FP version broke profiling on powerpc due to invalid results. > Casting to double instead of float resolved the issue, but with > Book-E not having a FP unit, the non-FP version looked preferrable. > Note that even on AIM hardware the FP version yielded an invalid > value for s_scale, so the problem is most likely with the compiler > or with the expression itself. Better use the integer code unconditionally if it works. There are minor advantages to never using FP in a program (it saves switching FP state in signal handlers on some arches, where the switch can involve up to 7 or 9 memory accesses to several hundred bytes of state each), but using FP in monstartup() causes every profiled program to use FP for most of its life. This is despite gprof using FP extensivly, so that FP needs to work for profiling to work. > Modified: head/lib/libc/gmon/gmon.c > ============================================================================== > --- head/lib/libc/gmon/gmon.c Wed Jun 30 01:10:08 2010 (r209603) > +++ head/lib/libc/gmon/gmon.c Wed Jun 30 01:40:25 2010 (r209604) > @@ -111,7 +111,7 @@ monstartup(lowpc, highpc) > > o = p->highpc - p->lowpc; > if (p->kcountsize < o) { > -#ifndef hp300 > +#if !defined(__powerpc__) The style regression. > s_scale = ((float)p->kcountsize / o ) * SCALE_1_TO_1; I can't see any bugs in this expression. p->kcountsize is < o, and the scale factor is only 65536, so there should be no problems with overflow. Using float instead of double is a almost pointless since the expression will be evaluated in double precision on most machines. powerpc claims that FLT_EVAL_METHOD is 1, so powerpc is one of these machines, so changing from float to double should have an especially small effect on it. So the bug is apparently in powerpc's conversion from u_long to float, or in promotion to double. I guess it is in conversion from u_long to float. > #else /* avoid floating point */ > int quot = o / p->kcountsize; > 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. Bruce