Summary: | [WinCairo] Enable JIT on 32-bit. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | peavo | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | alex.christensen, benjamin, bfulgham, cmarcelo, commit-queue, mark.lam, msaboff | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
peavo
2014-10-08 06:41:06 PDT
Created attachment 239472 [details]
Patch
This seems fine to me; it won't affect the Apple Windows port. Did the jit used to require sse2? Wasn't the jit enabled before? Couldn't we check to see if the CPU supports sse2 and enable it at runtime? Shouldn't the apple port do this, too? I don't think this is a good idea (In reply to comment #3) > Did the jit used to require sse2? Wasn't the jit enabled before? Couldn't we check to see if the CPU supports sse2 and enable it at runtime? Shouldn't the apple port do this, too? I don't think this is a good idea The LLInt and JITs use the SSE2 instructions for FP calculations. We could check at runtime (patch welcome), but this should only be on a 32 bit path. The Apple port doesn't need to check, all Intel Macs have new enough CPUs. (In reply to comment #4) > (In reply to comment #3) > > Did the jit used to require sse2? Wasn't the jit enabled before? Couldn't we check to see if the CPU supports sse2 and enable it at runtime? Shouldn't the apple port do this, too? I don't think this is a good idea > > The LLInt and JITs use the SSE2 instructions for FP calculations. We could check at runtime (patch welcome), but this should only be on a 32 bit path. > > The Apple port doesn't need to check, all Intel Macs have new enough CPUs. Would it be sufficient to do the same as in http://trac.webkit.org/changeset/167061, only adding a SSE2 check? I searched through the code generated by the LLInt, and didn't find any SSE2 instructions, only x87 floating point instructions, as far as I could see. (In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > Did the jit used to require sse2? Wasn't the jit enabled before? Couldn't we check to see if the CPU supports sse2 and enable it at runtime? Shouldn't the apple port do this, too? I don't think this is a good idea > > > > The LLInt and JITs use the SSE2 instructions for FP calculations. We could check at runtime (patch welcome), but this should only be on a 32 bit path. > > > > The Apple port doesn't need to check, all Intel Macs have new enough CPUs. > > Would it be sufficient to do the same as in http://trac.webkit.org/changeset/167061, only adding a SSE2 check? That would probably work. > I searched through the code generated by the LLInt, and didn't find any SSE2 instructions, only x87 floating point instructions, as far as I could see. My bad. I don't do much with the X87 part of the offline assembler. The JIT don't have the SSE / X87 switch. They only have an SSE2 path. Created attachment 239483 [details]
Patch
I don't think adding the llint to the 32bit wincairo port without it enabled in the 32bit AppleWin port would be worth the small performance gain for the large risk of someone breaking something and nobody noticing (In reply to comment #8) > I don't think adding the llint to the 32bit wincairo port without it enabled in the 32bit AppleWin port would be worth the small performance gain for the large risk of someone breaking something and nobody noticing In the latest patch there is no difference between AppleWin and WinCairo; both enable LLINT, but disable JIT runtime if SSE2 is not present. Comment on attachment 239483 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239483&action=review Looks good but please fix the ChangeLog comments. Thanks. > Source/JavaScriptCore/ChangeLog:8 > + Enable JIT on Windows 32-bit, but disable it runtime if SSE2 is not present. typo": disable it *at* runtime > Source/WTF/ChangeLog:8 > + Enable JIT on Windows 32-bit, but disable it runtime if SSE2 is not present. ditto. Please fix comment. Created attachment 239487 [details]
Patch
Comment on attachment 239487 [details]
Patch
r=me
(In reply to comment #12) > (From update of attachment 239487 [details]) > r=me Thanks for your help and review, guys :) Comment on attachment 239487 [details] Patch Clearing flags on attachment: 239487 Committed r174473: <http://trac.webkit.org/changeset/174473> All reviewed patches have been landed. Closing bug. |