Last Comment Bug 735699 - IonMonkey: GETELEM: Add a dense array IC
: IonMonkey: GETELEM: Add a dense array IC
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 734383
Blocks: IonSpeed
  Show dependency treegraph
 
Reported: 2012-03-14 08:45 PDT by Jan de Mooij [:jandem]
Modified: 2012-03-16 12:31 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (9.21 KB, patch)
2012-03-15 09:54 PDT, Jan de Mooij [:jandem]
no flags Details | Diff | Splinter Review
Patch (9.22 KB, patch)
2012-03-15 10:35 PDT, Jan de Mooij [:jandem]
dvander: review+
Details | Diff | Splinter Review

Description Jan de Mooij [:jandem] 2012-03-14 08:45:48 PDT
SS 3d-raytrace adds properties to some arrays, so these arrays are no longer dense (bug 586842). This means we can't use our dense array fast paths in some of the vector functions, even though most of the arrays are dense. We should add a dense array IC to handle these cases.

This IC should also help Kraken crypto-aes.
Comment 1 Jan de Mooij [:jandem] 2012-03-15 09:54:47 PDT
Created attachment 606269 [details] [diff] [review]
Patch

Applies on top of bug 734383. This does not yet handle the out-of-bounds or hole cases, but we can support these later.
Comment 2 Jan de Mooij [:jandem] 2012-03-15 10:35:17 PDT
Created attachment 606283 [details] [diff] [review]
Patch

Forgot to qref.
Comment 3 David Anderson [:dvander] 2012-03-15 18:49:26 PDT
Comment on attachment 606283 [details] [diff] [review]
Patch

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

::: js/src/ion/IonCaches.cpp
@@ +463,5 @@
> +    Label failures;
> +    MacroAssembler masm;
> +
> +    // Guard object is a dense array.
> +    Shape *shape = GetDenseArrayShape(cx, script->global());

Neat - is this to generate a faster guard than a clasp guard?

@@ +501,5 @@
> +
> +    // All failures flow to here.
> +    masm.bind(&hole);
> +    masm.pop(object());
> +    masm.bind(&failures);

Just to make sure: there's no way object() is used by out(), right? There was a bug where GETPROP could clobber its output this way.
Comment 4 Jan de Mooij [:jandem] 2012-03-16 12:31:05 PDT
http://hg.mozilla.org/projects/ionmonkey/rev/b96587f04076

(In reply to David Anderson [:dvander] from comment #3)
> 
> Neat - is this to generate a faster guard than a clasp guard?

It's what JM does, it's faster than a clasp guard because loading the class requires a few loads nowadays.

> 
> Just to make sure: there's no way object() is used by out(), right?

True, the object register is not "at-start" so the output is guaranteed to use different registers.

Note You need to log in before you can comment on or make changes to this bug.