584223 - Performance optimizations for sqrts via Math.pow

Status: RESOLVED DUPLICATE of bug 564548

(Core :: JavaScript Engine, enhancement)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This makes a small change to the Math.pow runtime code to check if the exponent is 0.5 or -0.5. In such cases, it calls sqrt (or 1.0/sqrt) instead of pow. This speeds up Sunspider's partial-sums on my laptop by about 10%.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: The patch

Here's the actual patch.

Comment 2

[Andreas Gal :gal]

Could you please also add this to the traceable native? (same file, search for pow)

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: Patch for traceable native as well

Did as Andreas suggested. Speedup with the tracer running is ~20% on partial-sums.

Comment 5

[Robert Sayre]

oop, different bug

Comment 6

[Andreas Gal :gal]

SQRTPD xmm1,xmm2/m128

we should use that for sqrt and also for this pow special case.

Comment 7

[Brendan Eich [:brendan]]

Rockin!

/be

Comment 8

[Andreas Gal :gal]

SQRTPD is 2x faster than FSQRT, which libc seems to use. Also, we can special case on trace for the -0.5 there.

Comment 9

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to comment #8)
> SQRTPD is 2x faster than FSQRT, which libc seems to use. Also, we can special
> case on trace for the -0.5 there.

I did a few benchmarks and I don't think it's worth the trouble.

1. I compared the performance of SQRTSD (the scalar version of SQRTPD) to FSQRT in the following loop: for (i=1..100000) { x += 1.0/sqrt(i); }. The SQRTSD version was 20% faster.

2. I translated the partial-sums benchmark to C and compiled it with gcc, comparing an SSE2 version to an x87 version. The x87 version was actually a little faster, although I don't know why.

Take this all with a grain of salt since my laptop has a pretty lame FPU. But then, lots of people have laptops.

Comment 10

[Nicholas Nethercote [inactive]]

Comment on attachment 462559 [details] [diff] [review]
Patch for traceable native as well

This is a small but clear win.  Let's not get bogged down in the asm;  I suggest filing a follow-up bug for that.

Any objections to my r+?

Comment 11

[Andreas Gal :gal]

r=me

Comment 12

[Boris Zbarsky [:bzbarsky]]

Does this just need to get landed at this point?  Did it fall through the cracks?

Comment 13

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Sorry, this change got folded in with bug 564548, which optimized pow in a different way.