+++ This bug was initially created as a clone of Bug #663485 +++ I had a WIP-patch for this in bug 663485, but filing a separate bug so that we can close the other bug and focus on the LICM part here.

I'm trying smaller patches in this bug. This hoists arr.length for typed arrays. The patch adds an arrayKind member to invariant entries, this seemed the best approach: especially for hoisted bounds checks (not in this patch) we can share most code for dense/typed arrays.

Comment on attachment 548276 [details] [diff] [review] Hoist length Review of attachment 548276 [details] [diff] [review]: ----------------------------------------------------------------- There are several places missed here where we scan the existing invariants and do tests based on arraySlot (entryRedundant, addHoistedCheck, invariantArraySlots, ...). Can you either have these check the arrayKind too, or just use separate kinds for the hoisted typed array length/slots? I would need to see the bounds check hoisting patch, but it seems to me like we should be able to reuse the code whether or not different kinds are used. Either way, InvariantEntry is definitely getting complicated enough that it should have better abstraction (shouldn't need fixing for this bug).

Comment on attachment 548276 [details] [diff] [review] Hoist length (In reply to comment #2) > There are several places missed here where we scan the existing invariants > and do tests based on arraySlot (entryRedundant, addHoistedCheck, > invariantArraySlots, ...). Can you either have these check the arrayKind > too, or just use separate kinds for the hoisted typed array length/slots? This patch only adds the arrayKind field to the .array struct, not the .check struct which is used by these functions. > I would need to see the bounds check hoisting patch, but it seems to me like > we should be able to reuse the code whether or not different kinds are used. My initial patch renamed INVARIANT_SLOTS to INVARIANT_DENSE_SLOTS and added INVARIANT_TYPED_SLOTS. Same for *LENGTH and BOUNDS_CHECK. The reason was that the only difference between a dense/typed array BOUNDS_CHECK is the code generation part but maybe it's better to be explicit about it and use another kind, will investigate.

Decided to use a single patch after all. I renamed INVARIANT_LENGTH to DENSE_ARRAY_LENGTH and added TYPED_ARRAY_LENGTH. This seemed more readable than INVARIANT_TYPED_LENGTH. Same for BOUNDS_CHECK and *SLOTS.

Comment on attachment 548873 [details] [diff] [review] Patch v2 Review of attachment 548873 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/methodjit/LoopState.cpp @@ +746,5 @@ > > for (unsigned i = 0; i < invariantEntries.length(); i++) { > InvariantEntry &entry = invariantEntries[i]; > + if ((entry.kind == InvariantEntry::DENSE_ARRAY_SLOTS || > + entry.kind == InvariantEntry::TYPED_ARRAY_SLOTS) && It would be good to have a short comment noting why no arrayKind check is necessary. @@ +824,5 @@ > > for (unsigned i = 0; i < invariantEntries.length(); i++) { > InvariantEntry &entry = invariantEntries[i]; > + if ((entry.kind == InvariantEntry::DENSE_ARRAY_LENGTH || > + entry.kind == InvariantEntry::TYPED_ARRAY_LENGTH) && Ditto

Pushed with comments added: http://hg.mozilla.org/projects/jaegermonkey/rev/235a8bfe2665