Bug 140110 – Fix Use details for op_create_lexical_environment and op_create_arguments

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

The current "Use" details for op_create_lexical_environment and op_create_arguments are wrong.  op_create_argument uses nothing instead of 1st operand (the output local).  op_create_lexical_environment uses its 2nd operand (the scope chain) instead of the 1st (the output local).

[Mark Lam]

Created attachment 244027 [details]
the patch.

[Mark Lam]

FYI, I've run this patch on the layout tests, JSC stress tests, and the JSC benchmarks (for stability ... I didn't check perf because it doesn't look like it should impact perf).  There were no regressions.

[Filip Pizlo]

Comment on attachment 244027 [details]
the patch.

A def isn't necessarily a use unless the instruction reads the old value of the variable. Is that true of the op_create_* opcodes?

[Mark Lam]

(In reply to comment #3)
> Comment on attachment 244027 [details]
> the patch.
> 
> A def isn't necessarily a use unless the instruction reads the old value of
> the variable. Is that true of the op_create_* opcodes?

Yes.  op_create_arguments does not read the old value.  Hence, I changed it to use none.  op_create_lexical_environment reads operand 2, not operand 1.  Hence, I changed it accordingly too.

I didn’t change the defs which are already correct.

[Filip Pizlo]

Comment on attachment 244027 [details]
the patch.

Test case?  Or is this benign?

[Filip Pizlo]

(In reply to comment #5)
> Comment on attachment 244027 [details]
> the patch.
> 
> Test case?  Or is this benign?

Seems like not using an operand that is used should cause OSR exit glitches.

[Mark Lam]

(In reply to comment #6)
> (In reply to comment #5)
> > Comment on attachment 244027 [details]
> > the patch.
> > 
> > Test case?  Or is this benign?
> 
> Seems like not using an operand that is used should cause OSR exit glitches.

I think it’s benign because:
1. The node that is used is always the result of GetScope.
2. The CreateActivation that uses it effectively follows immediately after the GetScope.
3. There is no opportunity for an OSR exit between the GetScope and the CreateActivation.

[WebKit Commit Bot]

Comment on attachment 244027 [details]
the patch.

Clearing flags on attachment: 244027

Committed r177981: <http://trac.webkit.org/changeset/177981>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Mark Lam]

The patch was wrong about op_create_arguments.  It does indeed use its 1st operand.  Reverted that part of the patch in r177994: <http://trac.webkit.org/r177994>.  rs=pizlo.