When investigating performance of the fbirds demo, I've realized that every time we have a const declaration, reading it will induce a lexical check. However, in es6 spec, it is clearly written that a const var declaration needs an initializer, so the lexical check is not needed in this case. We shouldn't add the MLexicalCheck when the lexical is a constant declaration. asm.js uses const variables *a lot*, so --no-asmjs code is affected.

So here's an example in which we can safely remove the lexical check: function f() { const x = 42; function g() { return x; } return g; } But Waldo explained me that this depends on the function g not being hoisted, and that there are plenty of cases where we need to check that x isn't uninitialized, for instance: function f() { g(); const x = 42; function g() { return x; } return g; } Is there anything we can do here? An analysis that there are no function calls between the function top and the const declaration would be very hackish, as Waldo duly noted on irc, but I can't think of anything better...

We can do better for Ion for sure. For local variables, SSA (hell, even what we do currently in the frontend) should be able to do a good job eliminating superfluous checks. The problem is upvars. However, patterns like using hoisted function statements, should, I hope, be very rare in the wild. In Ion, for upvars that need lexical checks, we could hoist *all* such checks into the prologue. If any of them fail, we don't raise the error, as doing so in the prologue would violate the spec, but instead bail out into Baseline, which would take care of throwing at the right place.

Consider the following micro-benchmark: function f() { const x = 42; function g() { var s = 0 for (var i = 0; i < 1000; i++) s += x; return s; } return g; } var func = f(); var s = 0; var b = Date.now(); for (var i = 200000; i--;) s += func(); console.log(Date.now() - b); Reference (without patch): 286ms With patch: 176ms (~38% speedup) Jan thought about a case where we would bailout constantly, so I'll make a second patch with a test case that provokes this case, to avoid it...

And the invalidation part, as requested by jandem. If we see a lexical check failure, we'll just invalidate the script and make the instruction not movable afterwards, preventing a bailout loop. And I even have a test case for that!

Moar benchmarking! ref with patch speedup asmjs asmjs-ubench/fbirds-native: 0.24 0.20 ~16% 0.16 asmjs-ubench/mandelbrot-native: 0.84 0.46 ~45% 0.40

Comment on attachment 8578714 [details] [diff] [review] 1.lexical.movable.patch Review of attachment 8578714 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. ::: js/src/jit/Lowering.cpp @@ +4073,5 @@ > MOZ_ASSERT(input->type() == MIRType_Value); > LLexicalCheck *lir = new(alloc()) LLexicalCheck(); > redefine(ins, input); > useBox(lir, LLexicalCheck::Input, input); > + assignSnapshot(lir, Bailout_UninitializedLexical); ThrowUninitializedLexical could also use this bailout reason. ::: js/src/jit/MIR.h @@ +6974,3 @@ > setResultType(MIRType_Value); > setResultTypeSet(input->resultTypeSet()); > + setMovable(); Nice. Could you also add congruentTo for MLexicalCheck to fold them?

Comment on attachment 8578714 [details] [diff] [review] 1.lexical.movable.patch Review of attachment 8578714 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/MIR.h @@ +6966,1 @@ > // Checks if a value is JS_UNINITIALIZED_LEXICAL, throwing if so. Forgot to add that this comment is now outdated. Could you update the comment saying that we only bail, and leaves it to Baseline to throw at the correct pc?

Comment on attachment 8578787 [details] [diff] [review] 2-invalidate.patch Review of attachment 8578787 [details] [diff] [review]: ----------------------------------------------------------------- LGTM,

Tested locally on x64 -- tests pass, this isn't arch specific, so daring not to push to try in this case... remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fbe012eea2e6 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2cb6af5972f8

https://hg.mozilla.org/mozilla-central/rev/fbe012eea2e6 https://hg.mozilla.org/mozilla-central/rev/2cb6af5972f8