Closed Bug 902722 Opened 8 years ago Closed 8 years ago

Guard typed arrays' shape instead of clasp in ICs

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: shu, Assigned: shu)

Details

Attachments

(1 file)

Currently they are guarded on their clasp, which requires extra care in the IC structure to not generate duplicate stubs. I suppose TAs with own properties are rare enough that guarding on shape is fine.
It looks like currently we actually generate duplicate stubs for typed arrays in GetElementIC. This should fix that too.
Assignee: general → shu
Attachment #787245 - Flags: review?(jdemooij)
Comment on attachment 787245 [details] [diff] [review]
bug902722-guard-ta-shape.patch

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

This patch is fine (guarding on the shape is faster than guarding on the class), but I don't understand why it would avoid duplicate stubs?

::: js/src/ion/IonCaches.cpp
@@ +2314,5 @@
> +    // Guard on the shape.
> +    Shape *shape = tarr->lastProperty();
> +    if (!shape)
> +        return false;
> +    masm.branchTestObjShape(Assembler::NotEqual, object, shape, &failures);

lastProperty() is always non-NULL I think. IonCaches.cpp has more lastProperty() NULL checks, feel free to remove these as well.
Attachment #787245 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #2)
> Comment on attachment 787245 [details] [diff] [review]
> bug902722-guard-ta-shape.patch
> 
> Review of attachment 787245 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This patch is fine (guarding on the shape is faster than guarding on the
> class), but I don't understand why it would avoid duplicate stubs?
> 

You're right, it doesn't really do anything to help duplicates.
https://hg.mozilla.org/mozilla-central/rev/f5d38a9eb834
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.