Bug 124251

Summary: Change callToJavaScript thunk into an offline assembled stub
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eflews.bot, gyuyoung.kim
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 116888    
Attachments:
Description Flags
Patch
eflews.bot: commit-queue-
Updated patch
buildbot: commit-queue-
Updated with speculative windows build fix
buildbot: commit-queue-
Patch with fix for windows
ggaren: review+, eflews.bot: commit-queue-
Patch for Landing none

Michael Saboff
Reported 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.
Attachments
Patch (34.00 KB, patch)
2013-11-12 17:53 PST, Michael Saboff 		eflews.bot: commit-queue-
DetailsFormatted DiffDiff
Updated patch (34.00 KB, patch)
2013-11-12 18:16 PST, Michael Saboff 		buildbot: commit-queue-
DetailsFormatted DiffDiff
Updated with speculative windows build fix (34.11 KB, patch)
2013-11-13 09:47 PST, Michael Saboff 		buildbot: commit-queue-
DetailsFormatted DiffDiff
Patch with fix for windows (37.85 KB, patch)
2013-11-13 12:09 PST, Michael Saboff 		ggaren: review+
eflews.bot: commit-queue-
DetailsFormatted DiffDiff
Patch for Landing (37.72 KB, patch)
2013-11-13 23:36 PST, Michael Saboff 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 1 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.
EFL EWS Bot
Comment 2 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
Michael Saboff
Comment 3 2013-11-12 18:16:47 PST
Created attachment 216760 [details] Updated patch Speculative fix for EFL build failure.
Build Bot
Comment 4 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
Michael Saboff
Comment 5 2013-11-13 09:47:45 PST
Created attachment 216815 [details] Updated with speculative windows build fix
Build Bot
Comment 6 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
Michael Saboff
Comment 7 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.
WebKit Commit Bot
Comment 8 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.
Geoffrey Garen
Comment 9 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.
EFL EWS Bot
Comment 10 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
Michael Saboff
Comment 11 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.
Michael Saboff
Comment 12 2013-11-13 23:37:25 PST
Committed r159276: <http://trac.webkit.org/changeset/159276>
Note You need to log in before you can comment on or make changes to this bug.