Bug 124251 - Change callToJavaScript thunk into an offline assembled stub
Summary: Change callToJavaScript thunk into an offline assembled stub
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks: 116888
  Show dependency treegraph
 
Reported: 2013-11-12 17:15 PST by Michael Saboff
Modified: 2013-11-13 23:37 PST (History)
3 users (show)

See Also:


Attachments
Patch (34.00 KB, patch)
2013-11-12 17:53 PST, Michael Saboff
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Updated patch (34.00 KB, patch)
2013-11-12 18:16 PST, Michael Saboff
buildbot: commit-queue-
Details | Formatted Diff | Diff
Updated with speculative windows build fix (34.11 KB, patch)
2013-11-13 09:47 PST, Michael Saboff
buildbot: commit-queue-
Details | Formatted Diff | Diff
Patch with fix for windows (37.85 KB, patch)
2013-11-13 12:09 PST, Michael Saboff
ggaren: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch for Landing (37.72 KB, patch)
2013-11-13 23:36 PST, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2013-11-12 17:15:56 PST
After r158751: <http://trac.webkit.org/changeset/158751>, JavaScript execution requires the ability to JIT a thunk to enter JavaScript.  The thunks callToJavaScript and throwNotCaught should be changed into offline assembled stubs.
Comment 1 Michael Saboff 2013-11-12 17:53:26 PST
Created attachment 216754 [details]
Patch

The changes for ARM64, MIPS and SH4 have not been compiled or tested.
Comment 2 EFL EWS Bot 2013-11-12 18:04:10 PST
Comment on attachment 216754 [details]
Patch

Attachment 216754 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/22868904
Comment 3 Michael Saboff 2013-11-12 18:16:47 PST
Created attachment 216760 [details]
Updated patch

Speculative fix for EFL build failure.
Comment 4 Build Bot 2013-11-12 19:03:10 PST
Comment on attachment 216760 [details]
Updated patch

Attachment 216760 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/22888894
Comment 5 Michael Saboff 2013-11-13 09:47:45 PST
Created attachment 216815 [details]
Updated with speculative windows build fix
Comment 6 Build Bot 2013-11-13 10:30:19 PST
Comment on attachment 216815 [details]
Updated with speculative windows build fix

Attachment 216815 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/22779954
Comment 7 Michael Saboff 2013-11-13 12:09:24 PST
Created attachment 216836 [details]
Patch with fix for windows

Since the LLInt doesn't work on Windows, I reverted the Windows stubs back to inline assembly in JITStubsX86.h and JITStubsMSVC64.asm.
Comment 8 WebKit Commit Bot 2013-11-13 12:11:50 PST
Attachment 216836 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/dfg/DFGDriver.cpp', u'Source/JavaScriptCore/jit/GPRInfo.h', u'Source/JavaScriptCore/jit/JITCode.cpp', u'Source/JavaScriptCore/jit/JITExceptions.cpp', u'Source/JavaScriptCore/jit/JITStubs.h', u'Source/JavaScriptCore/jit/JITStubsMSVC64.asm', u'Source/JavaScriptCore/jit/JITStubsX86.h', u'Source/JavaScriptCore/jit/ThunkGenerators.cpp', u'Source/JavaScriptCore/jit/ThunkGenerators.h', u'Source/JavaScriptCore/llint/LLIntThunks.h', u'Source/JavaScriptCore/llint/LowLevelInterpreter.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter64.asm', u'Source/JavaScriptCore/offlineasm/arm.rb', u'Source/JavaScriptCore/offlineasm/arm64.rb', u'Source/JavaScriptCore/offlineasm/instructions.rb', u'Source/JavaScriptCore/offlineasm/mips.rb', u'Source/JavaScriptCore/offlineasm/registers.rb', u'Source/JavaScriptCore/offlineasm/sh4.rb', u'Source/JavaScriptCore/offlineasm/x86.rb', u'Source/JavaScriptCore/runtime/VM.cpp', u'Source/JavaScriptCore/runtime/VM.h']" exit_code: 1
Source/JavaScriptCore/jit/JITStubsX86.h:222:  Extra space before [  [whitespace/braces] [5]
Source/JavaScriptCore/jit/JITStubsX86.h:223:  Extra space before [  [whitespace/braces] [5]
Total errors found: 2 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Geoffrey Garen 2013-11-13 12:57:29 PST
Comment on attachment 216836 [details]
Patch with fix for windows

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

r=me

Please test on 32bit, 64bit, and Windows before landing.

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:424
> +macro makeCallToJavaScript(entry, newCallFrame, previousCFR)

50% of this function is #ifdef'd, and it is confusingly named synonymously with doCallToJavaScript. So, I think you should just move the relevant code into the right places by hand, and remove this helper function.
Comment 10 EFL EWS Bot 2013-11-13 17:06:08 PST
Comment on attachment 216836 [details]
Patch with fix for windows

Attachment 216836 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/22600136
Comment 11 Michael Saboff 2013-11-13 23:36:51 PST
Created attachment 216903 [details]
Patch for Landing

Made the changes suggested.  Made minor fixes to the X86 windows stubs.

Built and ran JavaScript tests on Mac 32 & 64 and Windows 32.
Comment 12 Michael Saboff 2013-11-13 23:37:25 PST
Committed r159276: <http://trac.webkit.org/changeset/159276>