140110 2015-01-05 18:55:00 -0800 Fix Use details for op_create_lexical_environment and op_create_arguments 2015-01-06 14:00:23 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mark.lam mark.lam commit-queue fpizlo ggaren mhahnenb mmirman msaboff oliver oldest_to_newest 1058886 0 mark.lam 2015-01-05 18:55:00 -0800 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). 1058887 1 244027 mark.lam 2015-01-05 19:00:31 -0800 Created attachment 244027 the patch. 1058888 2 mark.lam 2015-01-05 19:04:09 -0800 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. 1058889 3 244027 fpizlo 2015-01-05 19:29:12 -0800 Comment on attachment 244027 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? 1058891 4 mark.lam 2015-01-05 19:46:04 -0800 (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. 1058894 5 244027 fpizlo 2015-01-05 20:10:38 -0800 Comment on attachment 244027 the patch. Test case? Or is this benign? 1058895 6 fpizlo 2015-01-05 20:11:25 -0800 (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. 1059030 7 mark.lam 2015-01-06 09:35:09 -0800 (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. 1059084 8 244027 commit-queue 2015-01-06 11:39:07 -0800 Comment on attachment 244027 the patch. Clearing flags on attachment: 244027 Committed r177981: <http://trac.webkit.org/changeset/177981> 1059085 9 commit-queue 2015-01-06 11:39:10 -0800 All reviewed patches have been landed. Closing bug. 1059152 10 mark.lam 2015-01-06 14:00:23 -0800 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. 244027 2015-01-05 19:00:31 -0800 2015-01-06 11:39:07 -0800 the patch. bug-140110.patch text/plain 2167 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTc3OTQ1KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBA CisyMDE1LTAxLTA1ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBG aXggVXNlIGRldGFpbHMgZm9yIG9wX2NyZWF0ZV9sZXhpY2FsX2Vudmlyb25tZW50IGFuZCBvcF9j cmVhdGVfYXJndW1lbnRzLgorICAgICAgICA8aHR0cHM6Ly93ZWJraXQub3JnL2IvMTQwMTEwPgor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBjdXJy ZW50ICJVc2UiIGRldGFpbHMgZm9yIG9wX2NyZWF0ZV9sZXhpY2FsX2Vudmlyb25tZW50IGFuZAor ICAgICAgICBvcF9jcmVhdGVfYXJndW1lbnRzIGFyZSB3cm9uZy4gIG9wX2NyZWF0ZV9hcmd1bWVu dCB1c2VzIG5vdGhpbmcgaW5zdGVhZCBvZiB0aGUKKyAgICAgICAgMXN0IG9wZXJhbmQgKHRoZSBv dXRwdXQgbG9jYWwpLiAgb3BfY3JlYXRlX2xleGljYWxfZW52aXJvbm1lbnQgdXNlcyBpdHMgMm5k CisgICAgICAgIG9wZXJhbmQgKHRoZSBzY29wZSBjaGFpbikgaW5zdGVhZCBvZiB0aGUgMXN0ICh0 aGUgb3V0cHV0IGxvY2FsKS4KKyAgICAgICAgVGhpcyBwYXRjaCBmaXhlcyB0aGVtIHRvIHNwZWNp ZnkgdGhlIHByb3BlciB1c2VzLgorCisgICAgICAgICogYnl0ZWNvZGUvQnl0ZWNvZGVVc2VEZWYu aDoKKyAgICAgICAgKEpTQzo6Y29tcHV0ZVVzZXNGb3JCeXRlY29kZU9mZnNldCk6CisKIDIwMTUt MDEtMDMgIE1pY2hhZWwgU2Fib2ZmICA8bXNhYm9mZkBhcHBsZS5jb20+CiAKICAgICAgICAgQ3Jh c2ggaW4gb3BlcmF0aW9uTmV3RnVuY3Rpb24gd2hlbiBzY3JvbGxpbmcgb24gR29vZ2xlKwpJbmRl eDogU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0J5dGVjb2RlVXNlRGVmLmgKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0J5dGVjb2RlVXNlRGVmLmgJ KHJldmlzaW9uIDE3NzkzMCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ieXRlY29kZS9CeXRl Y29kZVVzZURlZi5oCSh3b3JraW5nIGNvcHkpCkBAIC00MCw2ICs0MCw3IEBAIHZvaWQgY29tcHV0 ZVVzZXNGb3JCeXRlY29kZU9mZnNldCgKICAgICBPcGNvZGVJRCBvcGNvZGVJRCA9IGludGVycHJl dGVyLT5nZXRPcGNvZGVJRChpbnN0cnVjdGlvbi0+dS5vcGNvZGUpOwogICAgIHN3aXRjaCAob3Bj b2RlSUQpIHsKICAgICAvLyBObyB1c2VzLgorICAgIGNhc2Ugb3BfY3JlYXRlX2FyZ3VtZW50czoK ICAgICBjYXNlIG9wX25ld19yZWdleHA6CiAgICAgY2FzZSBvcF9uZXdfYXJyYXlfYnVmZmVyOgog ICAgIGNhc2Ugb3BfdGhyb3dfc3RhdGljX2Vycm9yOgpAQCAtNTUsOSArNTYsNyBAQCB2b2lkIGNv bXB1dGVVc2VzRm9yQnl0ZWNvZGVPZmZzZXQoCiAgICAgY2FzZSBvcF90b3VjaF9lbnRyeToKICAg ICBjYXNlIG9wX3Byb2ZpbGVfY29udHJvbF9mbG93OgogICAgICAgICByZXR1cm47Ci0gICAgY2Fz ZSBvcF9jcmVhdGVfbGV4aWNhbF9lbnZpcm9ubWVudDoKICAgICBjYXNlIG9wX2dldF9zY29wZToK LSAgICBjYXNlIG9wX2NyZWF0ZV9hcmd1bWVudHM6CiAgICAgY2FzZSBvcF90b190aGlzOgogICAg IGNhc2Ugb3BfcG9wX3Njb3BlOgogICAgIGNhc2Ugb3BfcHJvZmlsZV93aWxsX2NhbGw6CkBAIC0x MTQsNiArMTEzLDcgQEAgdm9pZCBjb21wdXRlVXNlc0ZvckJ5dGVjb2RlT2Zmc2V0KAogICAgICAg ICBmdW5jdG9yKGNvZGVCbG9jaywgaW5zdHJ1Y3Rpb24sIG9wY29kZUlELCBpbnN0cnVjdGlvbls0 XS51Lm9wZXJhbmQpOwogICAgICAgICByZXR1cm47CiAgICAgfQorICAgIGNhc2Ugb3BfY3JlYXRl X2xleGljYWxfZW52aXJvbm1lbnQ6CiAgICAgY2FzZSBvcF9nZXRfZW51bWVyYWJsZV9sZW5ndGg6 CiAgICAgY2FzZSBvcF9uZXdfZnVuY19leHA6CiAgICAgY2FzZSBvcF90b19pbmRleF9zdHJpbmc6 Cg==