Summary: | Fix Use details for op_create_lexical_environment and op_create_arguments | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||
Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, fpizlo, ggaren, mhahnenb, mmirman, msaboff, oliver | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Mark Lam
2015-01-05 18:55:00 PST
Created attachment 244027 [details]
the patch.
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. 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?
(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. Comment on attachment 244027 [details]
the patch.
Test case? Or is this benign?
(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. (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. Comment on attachment 244027 [details] the patch. Clearing flags on attachment: 244027 Committed r177981: <http://trac.webkit.org/changeset/177981> All reviewed patches have been landed. Closing bug. 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. |