777643 - JM: fix arguments.length jsop_getprop fast path

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Right now arguments.length will always go through stubs::GetProp, even when !script->needsArgsObj() since the types->isMagicArguments test is inside the top->mightBeType(JSVAL_TYPE_OBJECT) branch, which is going to be false for optimized arguments.

Comment 1

[Luke Wagner [:luke]]

Attachment: fix

This seems to give back 5% on earley-boyer.  This probably explains the regressions in bug 712714.

Comment 2

[Brian Hackett [Laid off!]]

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

Review of attachment 646326 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/methodjit/Compiler.cpp
@@ +4785,5 @@
>          }
>          return true;
>      }
>  
> +    /* Handle lenth accesses of optimize 'arguments'. */

typos

@@ +4788,5 @@
>  
> +    /* Handle lenth accesses of optimize 'arguments'. */
> +    if (name == cx->runtime->atomState.lengthAtom &&
> +        cx->typeInferenceEnabled() &&
> +        analysis->poppedTypes(PC, 0)->isMagicArguments(cx) &&

the poppedTypes call can just be 'top', right?

Comment 3

[Luke Wagner [:luke]]

I assumed there was some subtle difference between the type info in a FrameEntry and in poppedTypes since the code right below this uses both 'top' and 'types'.  Also, isMagicArguments is on the TypeSet.

Comment 4

[Luke Wagner [:luke]]

(Are they equivalent?  It isn't easy to see from the source.)

Comment 5

[Brian Hackett [Laid off!]]

Oops, you're right, poppedTypes is better to use here as top is a FrameEntry rather than a TypeSet.

Comment 6

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/01a7a9d46117

Comment 7

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/01a7a9d46117

Comment 8

[Brendan Eich [:brendan]]

What regressed this bug? Please set as blocked by this bug.

/be