Bug 138708 - Change X86/64 JIT probes to save/restore xmm regs as double instead of __m128
Summary: Change X86/64 JIT probes to save/restore xmm regs as double instead of __m128
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks: 138671
  Show dependency tree / graph
 
Reported: 2014-11-13 14:35 PST by Mark Lam
Modified: 2014-11-13 15:45 PST (History)
4 users (show)

See Also:

Attachments
the patch. (19.56 KB, patch)
2014-11-13 14:42 PST, Mark Lam 		msaboff: review+
Details | Formatted Diff | Diff
follow up patch. (1.68 KB, patch)
2014-11-13 15:38 PST, Mark Lam 		msaboff: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2014-11-13 14:35:58 PST 
The JIT code only uses the xmm regs as double registers.  This patch changes the storage types of the FP registers in X86Assembler.h to double instead of __m128, and updates the X86 and X86_64 JIT probe implementations accordingly.

Also made some minor cosmetic changes in the output of the probe dump functions.
Comment 1 Mark Lam 2014-11-13 14:42:36 PST 
Created attachment 241502 [details]
the patch.
Comment 2 Michael Saboff 2014-11-13 14:54:22 PST 
Comment on attachment 241502 [details]
the patch.

r=me
Comment 3 Mark Lam 2014-11-13 15:00:49 PST 
Thanks.  Landed in r176095: <http://trac.webkit.org/r176095>.
Comment 4 Joseph Pecoraro 2014-11-13 15:21:51 PST 
Comment on attachment 241502 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=241502&action=review

> Source/JavaScriptCore/assembler/X86Assembler.h:75
>  // the storing still expects a __m128 slot. This will be changed when the JIT
>  // probe code is updated later to reflect the JITs' usage of these registers.
>  #define FOR_EACH_CPU_FPREGISTER(V) \

Looks like the comment above this is now incorrect?
Comment 5 Mark Lam 2014-11-13 15:29:34 PST 
Comment on attachment 241502 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=241502&action=review

>> Source/JavaScriptCore/assembler/X86Assembler.h:75
>>  #define FOR_EACH_CPU_FPREGISTER(V) \
> 
> Looks like the comment above this is now incorrect?

Arggg ... thanks for catching this.  Will fix.
Comment 6 Mark Lam 2014-11-13 15:38:31 PST 
Created attachment 241512 [details]
follow up patch.
Comment 7 Michael Saboff 2014-11-13 15:41:40 PST 
Comment on attachment 241512 [details]
follow up patch.

r=me
Comment 8 Mark Lam 2014-11-13 15:45:08 PST 
Thanks for the review.  Follow up landed in r176100: <http://trac.webkit.org/r176100>.