code review 202042: math: special cases for Modf, Frexp and Ldexp; added Mo...

math: special cases for Modf, Frexp and Ldexp; added Modf_386

Also moved Modf from bits.go into modf.go and added timing tests.

[Charlie Dorian, 2010-02-04 05:19:14]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[rsc, 2010-02-04 08:20:17]

Thanks.

http://codereview.appspot.com/202042/diff/1001/14
File src/pkg/math/Makefile (right):

http://codereview.appspot.com/202042/diff/1001/14#newcode1
src/pkg/math/Makefile:1: # Copyright 2009-2010 The Go Authors. All rights
reserved.
undo -2010.

do put 2010 on new files, but don't worry about
updating old files.

http://codereview.appspot.com/202042/diff/1001/15
File src/pkg/math/all_test.go (right):

http://codereview.appspot.com/202042/diff/1001/15#newcode1
src/pkg/math/all_test.go:1: // Copyright 2009-2010 The Go Authors. All rights
reserved.
this might have been me but drop -2010

http://codereview.appspot.com/202042/diff/1001/15#newcode979
src/pkg/math/all_test.go:979: if f, j := Frexp(vf[i]); !(veryclose(frexp[i].f,
f) && frexp[i].i == j) {
distrbute ! when it's easier to read (almost always):

!veryclose || frexp != j

http://codereview.appspot.com/202042/diff/1001/15#newcode984
src/pkg/math/all_test.go:984: if f, j := Frexp(vffrexpSC[i]);
!(alike(frexpSC[i].f, f) && frexpSC[i].i == j) {
!alike || frexp.i != j

http://codereview.appspot.com/202042/diff/1001/15#newcode1071
src/pkg/math/all_test.go:1071: if f, g := Modf(vf[i]); !(veryclose(modf[i][0],
f) && veryclose(modf[i][1], g)) {
!veryclose || !veryclose

http://codereview.appspot.com/202042/diff/1001/15#newcode1076
src/pkg/math/all_test.go:1076: if f, g := Modf(vfmodfSC[i]);
!(alike(modfSC[i][0], f) && alike(modfSC[i][1], g)) {
!alike || !alike

http://codereview.appspot.com/202042/diff/1001/16
File src/pkg/math/bits.go (right):

http://codereview.appspot.com/202042/diff/1001/16#newcode1
src/pkg/math/bits.go:1: // Copyright 2009-2010 The Go Authors. All rights
reserved.
drop -2010

http://codereview.appspot.com/202042/diff/1001/17
File src/pkg/math/modf.go (right):

http://codereview.appspot.com/202042/diff/1001/17#newcode1
src/pkg/math/modf.go:1: // Copyright 2009-2010 The Go Authors. All rights
reserved.
drop -2010

http://codereview.appspot.com/202042/diff/1001/18
File src/pkg/math/modf_386.s (right):

http://codereview.appspot.com/202042/diff/1001/18#newcode9
src/pkg/math/modf_386.s:9: FSTCW   -2(SP)       // save old Control Word
all the control word fiddling is typically very slow.
is this really faster than the portable version?
seems like it would be much less noticeable than say Sin.

[Charlie Dorian, 2010-02-04 13:15:09]

Hello rsc (cc: golang-dev@googlegroups.com),

Please take another look.

[Charlie Dorian, 2010-02-04 13:38:23]

On Thu, Feb 4, 2010 at 3:20 AM, <rsc@golang.org> wrote:

> Thanks.
>
>
>
> http://codereview.appspot.com/202042/diff/1001/14
> File src/pkg/math/Makefile (right):
>
> http://codereview.appspot.com/202042/diff/1001/14#newcode1
> src/pkg/math/Makefile:1: # Copyright 2009-2010 The Go Authors. All
> rights reserved.
> undo -2010.
>
Done.

>
> do put 2010 on new files, but don't worry about
> updating old files.
>
> http://codereview.appspot.com/202042/diff/1001/15
> File src/pkg/math/all_test.go (right):
>
> http://codereview.appspot.com/202042/diff/1001/15#newcode1
> src/pkg/math/all_test.go:1: // Copyright 2009-2010 The Go Authors. All
> rights reserved.
> this might have been me but drop -2010
>
Done.

>
> http://codereview.appspot.com/202042/diff/1001/15#newcode979
> src/pkg/math/all_test.go:979: if f, j := Frexp(vf[i]);
> !(veryclose(frexp[i].f, f) && frexp[i].i == j) {
> distrbute ! when it's easier to read (almost always):
>
> !veryclose || frexp != j
>
Done.

>
> http://codereview.appspot.com/202042/diff/1001/15#newcode984
> src/pkg/math/all_test.go:984: if f, j := Frexp(vffrexpSC[i]);
> !(alike(frexpSC[i].f, f) && frexpSC[i].i == j) {
> !alike || frexp.i != j
>
Done.

>
> http://codereview.appspot.com/202042/diff/1001/15#newcode1071
> src/pkg/math/all_test.go:1071: if f, g := Modf(vf[i]);
> !(veryclose(modf[i][0], f) && veryclose(modf[i][1], g)) {
> !veryclose || !veryclose
>
Done.

>
> http://codereview.appspot.com/202042/diff/1001/15#newcode1076
> src/pkg/math/all_test.go:1076: if f, g := Modf(vfmodfSC[i]);
> !(alike(modfSC[i][0], f) && alike(modfSC[i][1], g)) {
> !alike || !alike
>
Done.

>
> http://codereview.appspot.com/202042/diff/1001/16
> File src/pkg/math/bits.go (right):
>
> http://codereview.appspot.com/202042/diff/1001/16#newcode1
> src/pkg/math/bits.go:1: // Copyright 2009-2010 The Go Authors. All
> rights reserved.
> drop -2010
>
Done.

>
> http://codereview.appspot.com/202042/diff/1001/17
> File src/pkg/math/modf.go (right):
>
> http://codereview.appspot.com/202042/diff/1001/17#newcode1
> src/pkg/math/modf.go:1: // Copyright 2009-2010 The Go Authors. All
> rights reserved.
> drop -2010
>
Done.

>
> http://codereview.appspot.com/202042/diff/1001/18
> File src/pkg/math/modf_386.s (right):
>
> http://codereview.appspot.com/202042/diff/1001/18#newcode9
> src/pkg/math/modf_386.s:9: FSTCW   -2(SP)       // save old Control Word
> all the control word fiddling is typically very slow.
> is this really faster than the portable version?
> seems like it would be much less noticeable than say Sin.

On a 1.0GHz Pentium III M, the 387 version is 17 percent faster (sine is 39
percent faster); on a 1.73GHz Pentium M, the 387 version is 4 percent faster
(sine is 44 percent faster). Yes, Control Word manipulation is slow and
ugly, but there's still some benefit.

It turns out to be easy to compare the non-387 and 387 versions: put a "#"
in front of the first 386 line of the Makefile (here, the one that says,
"asin_386.$O\"), then all the rest of the 386 stuff is ignored.  Run the
benchmarks with and without that "#" to see the differences.

>
>
> http://codereview.appspot.com/202042/show
>

[rsc, 2010-02-05 08:41:36]

*** Submitted as http://code.google.com/p/go/source/detail?r=6676dc48f19c ***

math: special cases for Modf, Frexp and Ldexp; added Modf_386

Also moved Modf from bits.go into modf.go and added timing tests.

R=rsc
CC=golang-dev
http://codereview.appspot.com/202042

Committer: Russ Cox <rsc@golang.org>