Bug 125653 - CStack Branch: Eliminate unnecessary add/sub 16 to stack pointer
Summary: CStack Branch: Eliminate unnecessary add/sub 16 to stack pointer
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:
 
Reported: 2013-12-12 14:05 PST by Michael Saboff
Modified: 2013-12-12 16:15 PST (History)
0 users

See Also:


Attachments
Patch (6.75 KB, patch)
2013-12-12 14:51 PST, Michael Saboff
ggaren: review+
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-12-12 14:05:16 PST
The DFG call out code constructs a new callee frame relative to the stack pointer.  Currently before making a call, it decrements the stack pointer by 16 to have it point above the returnPC and caller frame slots.  Instead, the stack pointer should be set properly at the top of the function so that the callee frame above the returnPC can be constructed relative to the stack pointer and then issue a call without adjusting the stack pointer.
Comment 1 Michael Saboff 2013-12-12 14:51:05 PST
Created attachment 219122 [details]
Patch
Comment 2 Michael Saboff 2013-12-12 14:52:21 PST
Landed in 160506 on the jsCStack branch: <http://trac.webkit.org/160506>
Comment 3 Geoffrey Garen 2013-12-12 15:30:57 PST
Comment on attachment 219122 [details]
Patch

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

r=me

> Source/JavaScriptCore/dfg/DFGGraph.cpp:706
> +    unsigned result = m_nextMachineLocal + std::max(m_parameterSlots, (unsigned)JSStack::CallerFrameAndPCSize);

static_cast, please.

> Source/JavaScriptCore/dfg/DFGGraph.cpp:707
>      result += result & 1; // Align the register count

This would be better expressed as "result = roundUpToMultipleOf<stackAlignment>(result);", and no comment.

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:44
> +const CallerFrameAndPCSize = 2 * PtrSize

Please add an ASSERT for this to LLIntData.cpp.
Comment 4 Michael Saboff 2013-12-12 16:15:59 PST
Changes from review landed in 160514 on the jsCStack branch: <http://trac.webkit.org/160514 >