592927 - nerf f.arguments so that we can optimize args like locals in JM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

In the general case, because of the arguments object, we have to assume every argument is an upvar in JM.  This means more loads/stores at stub calls and breaks copy propagation.  As an optimization, we might want to mark a script as statically not using 'arguments', however, this isn't good enough because of fun.arguments:

  function f(x) { /* no 'arguments' here */ g(); return x; }
  function g() { f.arguments[0] = "surprise!"; }
  assertEq(f(1), "surprise!")

Now, fun.arguments is unspecified by e262-v3 and a poison-pill on v5, so the only thing binding us here is the wild web.  v8 breaks f.arguments use if it doesn't see 'arguments' statically in a function.  I think JSC gets it right, but I didn't push to hard.  One thing we were talking about that would really help us is to say that any store to an arguments object where there was no static mention of 'arguments' in that function would be a nop.  This would be pretty easy to implement (in ArgSetter + a flag in the script).  Another option is to just return the poison pill for *any* reference to 'arguments' where 'arguments' is not syntactically present in the function.

What do you guys think?

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Specifically in v8, if |f| does not statically use the "arguments" keyword, writes to |f.arguments| will not be reflected in |f|'s formal arguments. They don't nop. This is true of JSC as well:

function f(x, y) { x(f); print(y); }
function g(x) { print(x.arguments[1]); x.arguments[1] = "bye"; }
f(g, "hello");

Prints "hello", "hello" in v8/JSC - "hello", "bye" in SpiderMonkey.

function f(x, y) { x(f); print(y); print(arguments[1]); }
function g(x) { print(x.arguments[1]); x.arguments[1] = "bye"; }
f(g, "hello");

Prints "hello", "bye" in all three engines.

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #0)
> In the general case, because of the arguments object, we have to assume every
> argument is an upvar in JM.

It's worse than an upvar as we've used the term. Dynamic scope!

> Now, fun.arguments is unspecified by e262-v3 and a poison-pill on v5,

ES5 strict mode only, not ES5 non-strict.

> What do you guys think?

I think we should match JSC and V8 (comment 1). Sooner is better. Need to test all combinations of interp and JITs.

/be

Comment 3

[Brendan Eich [:brendan]]

Someone with Windows should test Chakra (IE9) and report here.

/be

Comment 4

[Brendan Eich [:brendan]]

BTW, the closure optimizations (upvar analysis) considers formal parameters to be potentially aliased only if their function's body uses arguments (qualified by dot or not). So we are breaking a la JSC and V8 for upvar-param cases already.

/be

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

This ignores setting aliased arguments in an arguments object if (1) it has an active JSStackFrame, and (2) neither |eval| nor |arguments| were seen at compile time in the script.

This is the minimal change needed to let us optimize arguments in JM, and matches v8 functionality. We could, of course, remove f.arguments harder if desirable.

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 474882 [details] [diff] [review]
fix

Nice.

We can get rid of f.arguments next major release, maybe. It might take some time based on ES5 strict and successor (Harmony) versions built on it taking hold with devs. Not now.

/be

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/c156bf829377

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/c156bf829377