Attachment 219710 Details for Bug 126036 – the patch.

[patch] the patch.

bug-126036.patch (text/plain), 7.40 KB, created by Mark Lam on 2013-12-19 18:25:02 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2013-12-19 18:25:02 PST

Size: 7.40 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 160889)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,24 @@
>+2013-12-19  Mark Lam  <mark.lam@apple.com>
>+
>+        CStack: The JIT does not need a slow path stack check.
>+        https://bugs.webkit.org/show_bug.cgi?id=126036.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The JIT uses the C stack which is not growable. If we fail a stack
>+        check in the function header, then a stack overflow is imminent, and
>+        there's no need to redo the check in the slow path helper.
>+
>+        Hence, renamed operationStackCheck() to operationThrowStackOverflowError()
>+        and change the JIT and DFG code to use this appropriately.
>+
>+        * dfg/DFGJITCompiler.cpp:
>+        (JSC::DFG::JITCompiler::compileFunction):
>+        * jit/JIT.cpp:
>+        (JSC::JIT::privateCompile):
>+        * jit/JITOperations.cpp:
>+        * jit/JITOperations.h:
>+
> 2013-12-19  Michael Saboff  <msaboff@apple.com>
> 
>         CStack Branch: Fix call operationStackCheck to check the proper stack
>Index: Source/JavaScriptCore/dfg/DFGJITCompiler.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGJITCompiler.cpp	(revision 160882)
>+++ Source/JavaScriptCore/dfg/DFGJITCompiler.cpp	(working copy)
>@@ -327,9 +327,7 @@ void JITCompiler::compileFunction()
>     Label fromArityCheck(this);
>     // Plant a check that sufficient space is available in the JSStack.
>     addPtr(TrustedImm32(virtualRegisterForLocal(m_graph.requiredRegisterCountForExecutionAndExit() - 1).offset() * sizeof(Register)), GPRInfo::callFrameRegister, GPRInfo::regT1);
>-    Jump stackCheck = branchPtr(Above, AbsoluteAddress(m_vm->addressOfJSStackLimit()), GPRInfo::regT1);
>-    // Return here after stack check.
>-    Label fromStackCheck = label();
>+    Jump stackOverflow = branchPtr(Above, AbsoluteAddress(m_vm->addressOfJSStackLimit()), GPRInfo::regT1);
> 
>     checkStackPointerAlignment();
> 
>@@ -344,17 +342,15 @@ void JITCompiler::compileFunction()
> 
>     // === Function footer code generation ===
>     //
>-    // Generate code to perform the slow stack check (if the fast one in
>+    // Generate code to perform the stack overflow handling (if the stack check in
>     // the function header fails), and generate the entry point with arity check.
>     //
>-    // Generate the stack check; if the fast check in the function head fails,
>-    // we need to call out to a helper function to check whether more space is available.
>-    // FIXME: change this from a cti call to a DFG style operation (normal C calling conventions).
>-    stackCheck.link(this);
>+    // Generate the stack overflow handling; if the stack check in the function head fails,
>+    // we need to call out to a helper function to throw the StackOverflowError.
>+    stackOverflow.link(this);
> 
>     emitStoreCodeOrigin(CodeOrigin(0));
>-    m_speculative->callOperationWithCallFrameRollbackOnException(operationStackCheck, m_codeBlock);
>-    jump(fromStackCheck);
>+    m_speculative->callOperationWithCallFrameRollbackOnException(operationThrowStackOverflowError, m_codeBlock);
>     
>     // The fast entry point into a function does not check the correct number of arguments
>     // have been passed to the call (we only use the fast entry point where we can statically
>Index: Source/JavaScriptCore/jit/JIT.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/JIT.cpp	(revision 160882)
>+++ Source/JavaScriptCore/jit/JIT.cpp	(working copy)
>@@ -519,7 +519,7 @@ CompilationResult JIT::privateCompile(JI
>     sampleInstruction(m_codeBlock->instructions().begin());
> #endif
> 
>-    Jump stackCheck;
>+    Jump stackOverflow;
>     if (m_codeBlock->codeType() == FunctionCode) {
> #if ENABLE(VALUE_PROFILER)
>         ASSERT(m_bytecodeOffset == (unsigned)-1);
>@@ -542,11 +542,9 @@ CompilationResult JIT::privateCompile(JI
> #endif
> 
>         addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, regT1);
>-        stackCheck = branchPtr(Above, AbsoluteAddress(m_vm->addressOfJSStackLimit()), regT1);
>+        stackOverflow = branchPtr(Above, AbsoluteAddress(m_vm->addressOfJSStackLimit()), regT1);
>     }
> 
>-    Label functionBody = label();
>-
>     checkStackPointerAlignment();
>     addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
>     checkStackPointerAlignment();
>@@ -560,14 +558,9 @@ CompilationResult JIT::privateCompile(JI
> 
>     Label arityCheck;
>     if (m_codeBlock->codeType() == FunctionCode) {
>-        stackCheck.link(this);
>+        stackOverflow.link(this);
>         m_bytecodeOffset = 0;
>-        // FIXME: CStack - This may need to have some stack space allocated to make the call
>-        callOperationWithCallFrameRollbackOnException(operationStackCheck, m_codeBlock);
>-#ifndef NDEBUG
>-        m_bytecodeOffset = (unsigned)-1; // Reset this, in order to guard its use with ASSERTs.
>-#endif
>-        jump(functionBody);
>+        callOperationWithCallFrameRollbackOnException(operationThrowStackOverflowError, m_codeBlock);
> 
>         arityCheck = label();
>         store8(TrustedImm32(0), &m_codeBlock->m_shouldAlwaysBeInlined);
>Index: Source/JavaScriptCore/jit/JITOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/JITOperations.cpp	(revision 160882)
>+++ Source/JavaScriptCore/jit/JITOperations.cpp	(working copy)
>@@ -70,7 +70,7 @@ void * _ReturnAddress(void);
> #endif
> 
> 
>-void JIT_OPERATION operationStackCheck(ExecState* exec, CodeBlock* codeBlock)
>+void JIT_OPERATION operationThrowStackOverflowError(ExecState* exec, CodeBlock* codeBlock)
> {
>     // We pass in our own code block, because the callframe hasn't been populated.
>     VM* vm = codeBlock->vm();
>@@ -79,16 +79,7 @@ void JIT_OPERATION operationStackCheck(E
>         callerFrame = exec;
> 
>     NativeCallFrameTracer tracer(vm, callerFrame);
>-
>-#if ENABLE(LLINT_CLOOP)
>-    JSStack& stack = vm->interpreter->stack();
>-
>-    if (UNLIKELY(!stack.grow(&exec->registers()[codeBlock->stackPointerOffset()])))
>-        vm->throwException(callerFrame, createStackOverflowError(callerFrame));
>-#else
>-    if (!exec->vm().isSafeToRecurse(codeBlock->frameRegisterCount() * sizeof(Register)))
>-        vm->throwException(callerFrame, createStackOverflowError(callerFrame));
>-#endif // ENABLE(LLINT_CLOOP)
>+    vm->throwException(callerFrame, createStackOverflowError(callerFrame));
> }
> 
> int32_t JIT_OPERATION operationCallArityCheck(ExecState* exec)
>Index: Source/JavaScriptCore/jit/JITOperations.h
>===================================================================
>--- Source/JavaScriptCore/jit/JITOperations.h	(revision 160882)
>+++ Source/JavaScriptCore/jit/JITOperations.h	(working copy)
>@@ -187,7 +187,7 @@ typedef JSString* JIT_OPERATION (*Jss_JI
> void JIT_OPERATION lookupExceptionHandler(ExecState*) WTF_INTERNAL;
> void JIT_OPERATION operationVMHandleException(ExecState*) WTF_INTERNAL;
> 
>-void JIT_OPERATION operationStackCheck(ExecState*, CodeBlock*) WTF_INTERNAL;
>+void JIT_OPERATION operationThrowStackOverflowError(ExecState*, CodeBlock*) WTF_INTERNAL;
> int32_t JIT_OPERATION operationCallArityCheck(ExecState*) WTF_INTERNAL;
> int32_t JIT_OPERATION operationConstructArityCheck(ExecState*) WTF_INTERNAL;
> EncodedJSValue JIT_OPERATION operationGetById(ExecState*, StructureStubInfo*, EncodedJSValue, StringImpl*) WTF_INTERNAL;

Flags:

ggaren: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 126036: 219710