Bug 15617

Summary: improve speed of integer conversions
Product: WebKit Reporter: Darin Adler <darin>
Component: JavaScriptCoreAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Enhancement    
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
patch with change log
none
patch with change log
none
patch with change log (newer version with fixed .exp file) eric: review+

Description Darin Adler 2007-10-22 07:31:19 PDT
I was able to get a 1% speedup on SunSpider by eliminating an unnecessary float/double conversion.
Comment 1 Darin Adler 2007-10-22 07:36:43 PDT
Created attachment 16798 [details]
patch with change log
Comment 2 Mark Rowe (bdash) 2007-10-22 07:51:49 PDT
This looks nearly identical to Eric's patch on bug 15607.
Comment 3 Darin Adler 2007-10-22 10:05:00 PDT
I have a better version in the works.
Comment 4 Darin Adler 2007-10-22 10:15:39 PDT
Created attachment 16802 [details]
patch with change log
Comment 5 Darin Adler 2007-10-22 10:26:42 PDT
Created attachment 16803 [details]
patch with change log (newer version with fixed .exp file)
Comment 6 Eric Seidel (no email) 2007-10-22 10:49:28 PDT
Comment on attachment 16803 [details]
patch with change log (newer version with fixed .exp file)

Looks great.  I assume you ran the tests.  Also, (pot calling the kettle black here) I think it would be nice to replace all your float special values 4294967296.0, etc with some nice names (to make the code make sense to someone who didn't just hack on it for a couple hours. :)  (Also, nice catch with toInt32SlowCase calling toUInt32.  I'm curious if that would ever be "wrong" (such that we could build a test).  I guess it just changes what values could be stored in the float, since it's just doing the sign wrap at a different time.
Comment 7 Darin Adler 2007-10-22 10:57:10 PDT
(In reply to comment #6)
> (Also, nice catch with
> toInt32SlowCase calling toUInt32.  I'm curious if that would ever be "wrong"
> (such that we could build a test).

It wasn't wrong in the sense of "buggy". It was just inefficient for negative numbers.
Comment 8 Darin Adler 2007-10-22 10:58:40 PDT
(In reply to comment #6)
> I think it would be nice to replace all your float special values
> 4294967296.0, etc with some nice names (to make the code make sense to someone
> who didn't just hack on it for a couple hours. :)

I'll consider that. I'm worried that if I use const double or const float that it will generate different code, so it's a change I need to make carefully with measurements.