Bug 142153 - BytecodeGenerator shouldn't emit op_resolve_scope as a roundabout way of returning the scopeRegister
Summary: BytecodeGenerator shouldn't emit op_resolve_scope as a roundabout way of retu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 141174
  Show dependency tree / graph
 
Reported: 2015-03-01 12:52 PST by Filip Pizlo
Modified: 2015-03-01 18:09 PST (History)
0 users

See Also:

Attachments
the patch (17.43 KB, patch)
2015-03-01 12:53 PST, Filip Pizlo 		msaboff: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2015-03-01 12:52:17 PST 
Currently sometimes BytecodeGenerator will emit an op_resolve_scope with the LocalClosureVar mode and 0 as the scope depth, which ultimately gets turned into an operation that just moves the scopeRegister() to the result.  In cases like that, BytecodeGenerator should just use the scopeRegister() directly and not emit anything for scope resolution.
Comment 1 Filip Pizlo 2015-03-01 12:53:15 PST 
Created attachment 247637 [details]
the patch

Not quite ready for review yet, still need to run debug tests.
Comment 2 Filip Pizlo 2015-03-01 13:04:24 PST 
Running more tests now.  It's looking good.  I will run debug and release, 32-bit and 64-bit, before landing.
Comment 3 Michael Saboff 2015-03-01 13:14:06 PST 
Comment on attachment 247637 [details]
the patch

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

r=me
Pending tests pass and add ChangeLog description

> Source/JavaScriptCore/ChangeLog:7
> +

Please describe what the patch does?
Comment 4 Filip Pizlo 2015-03-01 13:19:58 PST 
(In reply to comment #3)
> Comment on attachment 247637 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247637&action=review
> 
> r=me
> Pending tests pass and add ChangeLog description
> 
> > Source/JavaScriptCore/ChangeLog:7
> > +
> 
> Please describe what the patch does?

Yup, added locally.
Comment 5 Filip Pizlo 2015-03-01 18:09:20 PST 
Landed in http://trac.webkit.org/changeset/180875