Bug 138252

Summary: Add scope operand to op_push_with_scope, op_push_name_scope and op_pop_scope
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 136724, 138254    
Attachments:
Description Flags
Patch
ggaren: review-
Patch with suggested updates ggaren: review+

Michael Saboff
Reported 2014-10-31 11:05:42 PDT
This is part of the effort to remove the scope chain from the call frame header (https://bugs.webkit.org/show_bug.cgi?id=136724). This bug is to add a source scope register index to the op_push_with_scope, op_push_name_scope and op_pop_scope byte codes. The new operand will be filled in but not used.
Attachments
Patch (18.78 KB, patch)
2014-10-31 11:23 PDT, Michael Saboff 		ggaren: review-
DetailsFormatted DiffDiff
Patch with suggested updates (20.31 KB, patch)
2014-10-31 12:46 PDT, Michael Saboff 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 1 2014-10-31 11:23:54 PDT
Created attachment 240746 [details] Patch
Geoffrey Garen
Comment 2 2014-10-31 11:37:44 PDT
Comment on attachment 240746 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240746&action=review > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2199 > +void BytecodeGenerator::emitComplexPopScopes(RegisterID* scopeReg, ControlFlowContext* topScope, ControlFlowContext* bottomScope) Let's call this "scope", or, if we need to disambiguate from something else named "scope", then "scopeRegister". > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2314 > +void BytecodeGenerator::emitPopScopes(RegisterID* scopeReg, int targetScopeDepth) Ditto. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:543 > + void emitPopScope(RegisterID* scopeReg); This should be "srcDst" to match other functions' naming. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:2253 > + RegisterID scopeDest(JSStack::ScopeChain); Let's make the scope register a data member in the BytecodeGenerator instead of a one-off allocation at the call site. It's not really valid to allocate bytecode registers in this way.
Michael Saboff
Comment 3 2014-10-31 12:46:01 PDT
Created attachment 240749 [details] Patch with suggested updates
Geoffrey Garen
Comment 4 2014-10-31 13:29:03 PDT
Comment on attachment 240749 [details] Patch with suggested updates r=me
Michael Saboff
Comment 5 2014-10-31 14:27:03 PDT
Committed r175426: <http://trac.webkit.org/changeset/175426>
Note You need to log in before you can comment on or make changes to this bug.