NetBSD Problem Report #49083

From www@NetBSD.org  Thu Aug  7 07:29:31 2014
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id CC082A99E7
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  7 Aug 2014 07:29:31 +0000 (UTC)
Message-Id: <20140807072930.0916AA9A3D@mollari.NetBSD.org>
Date: Thu,  7 Aug 2014 07:29:30 +0000 (UTC)
From: isaki@pastel-flower.jp
Reply-To: isaki@pastel-flower.jp
To: gnats-bugs@NetBSD.org
Subject: redundant codes in m68k s_ceil/s_floor
X-Send-Pr-Version: www-1.0

>Number:         49083
>Category:       lib
>Synopsis:       redundant codes in m68k s_ceil/s_floor
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 07 07:30:00 +0000 2014
>Closed-Date:    Sun Aug 10 13:40:59 +0000 2014
>Last-Modified:  Sun Aug 10 13:40:59 +0000 2014
>Originator:     Tetsuya Isaki
>Release:        NetBSD-current
>Organization:
>Environment:
NetBSD x68k.XXXXX 6.99.40 NetBSD 6.99.40 (GENERIC) #0: Thu Apr 17 13:40:12 JST 2014  isaki@XXXXXX:/var/obj/first/x68k/obj/sys/arch/x68k/compile/GENERIC x68k
>Description:
src/lib/libm/arch/m68k/{s_ceil,s_floor}.S seems to have some
redundant codes.

1)
  For s_ceil, I think FPCR should be "Round to +Inf"(0x30) always
  regardless of the sign of the input value, and the condition
  branch is not needed.  For s_floor, FPCR should be
  "Round to -Inf"(0x20) always as in the case of s_ceil.

  If they needs really such a special treatment (for example, some
  old chips have an errata), should write a note in comment.


2)
  I think that "fintd %sp@(4),%fp0" can be replaced by "fintx %fp0"
  in both of s_ceil and s_floor.  %fp0 already contains %sp@(4) here,
  so "fintx %fp0" is faster because it does not access memory.

Please review.

The attached patches pass tests/lib/libm/t_ceil on XM6i (which supports
68881 emulation).  Of course I know that it is not enough test,
but unfortunately I have no runnable real m68k machines now :(
>How-To-Repeat:
see lib/libm/arch/m68k/{s_ceil.S,s_floor.S}
>Fix:
1) redundant condition branch
Index: lib/libm/arch/m68k/s_ceil.S
===================================================================
RCS file: /cvsroot/src/lib/libm/arch/m68k/s_ceil.S,v
retrieving revision 1.7
diff -u -r1.7 s_ceil.S
--- lib/libm/arch/m68k/s_ceil.S	7 Aug 2003 16:44:40 -0000	1.7
+++ lib/libm/arch/m68k/s_ceil.S	7 Aug 2014 06:34:07 -0000
@@ -41,18 +41,12 @@
 RCSID("$NetBSD: s_ceil.S,v 1.7 2003/08/07 16:44:40 agc Exp $")

 | ceil(x)
-| -floor(-x), for all real x
 ENTRY(ceil)
 	fmovel	%fpcr,%d0	| save old FPCR
 	fmoved	%sp@(4),%fp0	| get argument
 	fbun	Lret		| if NaN, return NaN
-	fbolt	Lrtz		| <0, round to zero
-	fmovel	#0x30,%fpcr	| >=0, round to inf
-	jra	Ldoit
-Lrtz:
-	fmovel	#0x10,%fpcr
-Ldoit:
-	fintd	%sp@(4),%fp0	| truncate
+	fmovel	#0x30,%fpcr	| round to +inf
+	fintd	%sp@(4),%fp0		| truncate
 	fmovel	%d0,%fpcr	| restore old FPCR
 Lret:
 #ifndef __SVR4_ABI__
Index: lib/libm/arch/m68k/s_floor.S
===================================================================
RCS file: /cvsroot/src/lib/libm/arch/m68k/s_floor.S,v
retrieving revision 1.7
diff -u -r1.7 s_floor.S
--- lib/libm/arch/m68k/s_floor.S	7 Aug 2003 16:44:41 -0000	1.7
+++ lib/libm/arch/m68k/s_floor.S	7 Aug 2014 06:34:07 -0000
@@ -46,12 +46,7 @@
 	fmovel	%fpcr,%d0	| save old FPCR
 	fmoved	%sp@(4),%fp0	| get argument
 	fbun	Lret		| if NaN, return NaN
-	fboge	Lrtz		| >=0, round to zero
-	fmovel	#0x20,%fpcr	| <0, round to -inf
-	jra	Ldoit
-Lrtz:
-	fmovel	#0x10,%fpcr
-Ldoit:
+	fmovel	#0x20,%fpcr	| round to -inf
 	fintd	%sp@(4),%fp0	| truncate
 	fmovel	%d0,%fpcr	| restore old FPCR
 Lret:


2) redundant memory access
--- lib/libm/arch/m68k/s_ceil.S.1	2014-08-07 15:51:06.000000000 +0900
+++ lib/libm/arch/m68k/s_ceil.S	2014-08-07 15:51:37.000000000 +0900
@@ -46,7 +46,7 @@
 	fmoved	%sp@(4),%fp0	| get argument
 	fbun	Lret		| if NaN, return NaN
 	fmovel	#0x30,%fpcr	| round to +inf
-	fintd	%sp@(4),%fp0	| truncate
+	fintx	%fp0		| truncate
 	fmovel	%d0,%fpcr	| restore old FPCR
 Lret:
 #ifndef __SVR4_ABI__
--- lib/libm/arch/m68k/s_floor.S.1	2014-08-04 14:50:54.000000000 +0900
+++ lib/libm/arch/m68k/s_floor.S	2014-08-07 15:51:48.000000000 +0900
@@ -47,7 +47,7 @@
 	fmoved	%sp@(4),%fp0	| get argument
 	fbun	Lret		| if NaN, return NaN
 	fmovel	#0x20,%fpcr	| round to -inf
-	fintd	%sp@(4),%fp0	| truncate
+	fintx	%fp0		| truncate
 	fmovel	%d0,%fpcr	| restore old FPCR
 Lret:
 #ifndef __SVR4_ABI__

>Release-Note:

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/49083: redundant codes in m68k s_ceil/s_floor
Date: Sun, 10 Aug 2014 12:04:21 +0200

 With this patch applied, I tested on mac68k with 68040 cpu:

 NetBSD 6.99.49 (MAC-BETH) #1: Fri Aug  8 12:09:01 CEST 2014
         martin@night-owl.duskware.de:/usr/src/sys/arch/mac68k/compile/MAC-BETH
 Apple Macintosh Centris 660AV  (68040)
 cpu: delay factor 800
 fpu: mc68040
 total memory = 45056 KB
 avail memory = 40304 KB

 and got for "atf-run t_ceil | atf-report":

 Summary for 1 test programs:
     54 passed test cases.
     0 failed test cases.
     0 expected failed test cases.
     0 skipped test cases.


 Looks good.

 Martin

From: "Tetsuya Isaki" <isaki@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49083 CVS commit: src/lib/libm/arch/m68k
Date: Sun, 10 Aug 2014 13:29:10 +0000

 Module Name:	src
 Committed By:	isaki
 Date:		Sun Aug 10 13:29:10 UTC 2014

 Modified Files:
 	src/lib/libm/arch/m68k: s_ceil.S s_floor.S

 Log Message:
 Remove a meaningless conditional branch.
 PR/49083 (1)


 To generate a diff of this commit:
 cvs rdiff -u -r1.7 -r1.8 src/lib/libm/arch/m68k/s_ceil.S \
     src/lib/libm/arch/m68k/s_floor.S

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Tetsuya Isaki" <isaki@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49083 CVS commit: src/lib/libm/arch/m68k
Date: Sun, 10 Aug 2014 13:30:43 +0000

 Module Name:	src
 Committed By:	isaki
 Date:		Sun Aug 10 13:30:43 UTC 2014

 Modified Files:
 	src/lib/libm/arch/m68k: s_ceil.S s_floor.S

 Log Message:
 Improve the code.  %sp@(4) is already loaded in %fp0.
 PR/49083 (2)


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/lib/libm/arch/m68k/s_ceil.S \
     src/lib/libm/arch/m68k/s_floor.S

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: isaki@NetBSD.org
State-Changed-When: Sun, 10 Aug 2014 13:40:59 +0000
State-Changed-Why:
committed.


>Unformatted:

Home	PR Database Search