826691 - BaselineCompiler: Add optimized SETPROP stub

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

Adds a stub to handle SETPROP/SETGNAME/SETNAME of (existing) native object properties. This patch also changes the type update fallback stub, we now pass the object on which we are setting the property. This is necessary so that we can use the TI AddTypePropertyId functions (using TypeObject::addPropertyType directly is wrong if, for instance, TI is disabled).

With this patch we have some new jit-test failures, because SETPROP requires a write barrier during incremental GC. I will attach a patch for that next week (and I won't land this patch before that's done).

Comment 2

[Kannan Vijayan [:djvj]]

Comment on attachment 697979 [details] [diff] [review]
Patch

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

::: js/src/ion/BaselineIC.cpp
@@ +1899,5 @@
> +
> +    // Guard that the type object matches.
> +    masm.loadPtr(Address(BaselineStubReg, ICSetProp_Native::offsetOfType()), scratch);
> +    masm.branchPtr(Assembler::NotEqual, Address(objReg, JSObject::offsetOfType()), scratch,
> +                   &failureUnstow);

The EmitStowICValues could be moved to happen after the guard that follows (changing the branchPtr to jump to |&failure| instead of |&failureUnstow|).  In the case of where the guard fails, this saves us an extra stow and and an unstow.  Moreover, it brings the EmitStowICValues and EmitUnstowICValues calls closer together, which helps clarity.

::: js/src/ion/BaselineIC.h
@@ +468,5 @@
> +    inline ICFallbackStub *getFallbackStub() {
> +        ICStub *stub = this;
> +        while (!stub->isFallback())
> +            stub = stub->next();
> +        return stub->toFallbackStub();

nit: JS_ASSERT(!stub->next()) before return.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 2: Add prebarriers

Add incremental GC barriers; fixes the test failures I got with the previous patch. With the (disabled) barriers, I didn't notice any slowdown, so this always adds the patchable barriers to the stubs. This is nice because it makes toggling barriers very fast, since we don't have to walk over scripts/IC's.

Comment 4

[Jan de Mooij [:jandem]]

Pushed with nits addressed:

https://hg.mozilla.org/projects/ionmonkey/rev/6a3cf1c0b0fb
https://hg.mozilla.org/projects/ionmonkey/rev/b2be0e8a374f