Closed
Bug 701964
Opened 13 years ago
Closed 13 years ago
IonMonkey: Compile JSOP_LENGTH
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: nbp, Assigned: nbp)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
25.53 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
Necessary for benchmarks.
Assignee | ||
Comment 1•13 years ago
|
||
Implement JSOP_LENGTH for Strings and Arrays. Fallback to JSOP_GETPROP if the the type oracle does cannot guess the type.
Attachment #581393 -
Flags: review?(dvander)
Comment on attachment 581393 [details] [diff] [review]
Implement JSOP_LENGTH.
Review of attachment 581393 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good overall, but I have a few comments on code sharing and TI integration so I'd like to see the next iteration.
::: js/src/ion/IonBuilder.cpp
@@ +2871,5 @@
> +IonBuilder::jsop_length()
> +{
> + TypeOracle::Unary signature = oracle->unaryOp(script, pc);
> +
> + switch (signature.ival) {
Since the fast-paths in here are property accesses, there are special TI rules in play. For now it suffices to only build these fast-paths if the output type is known to be int32. (In TI, the output type set could be empty, which means it has not expected this code to run yet.)
@@ +2881,5 @@
> + return true;
> + }
> +
> + case MIRType_Object:
> + if (oracle->elementReadIsDense(script, pc)) {
Here, I think it's better to introduce a new Oracle API rather than extend "elementReadIsDense". You could include the popped-TypeSet in TypeOracle::Unary, and then check:
> if (signature.inTypes &&
> !signature.inTypes->hasObjectFlags(cx, types::OBJECT_FLAG_NON_DENSE_ARRAY))
> {
@@ +2893,5 @@
> + current->push(length);
> +
> + return true;
> + } else
> + return jsop_getprop(info().getAtom(pc));
Nit: brace this, since the other half is braced.
::: js/src/ion/x86/CodeGenerator-x86.cpp
@@ +443,5 @@
> + Register output = ToRegister(lir->output());
> +
> + masm.loadPtr(lengthAndFlags, output);
> + masm.shrl(Imm32(JSString::LENGTH_SHIFT), output);
> + return true;
Let's hoist this outside of CG-$(ARCH) and into CodeGenerator.cpp - just add an "lshiftPtr(Imm32 amount, Register reg)" abstraction to all the MacroAssemblers.
Attachment #581393 -
Flags: review?(dvander) → review+
Updated•13 years ago
|
Attachment #581393 -
Flags: review+
Assignee | ||
Comment 3•13 years ago
|
||
Factor visitStringLength in ion/CG. (Add rshiftPtr in each MacroAssembler)
Add test case checking with type inference.
Add UnaryTypes and used it instead of Unary structure. (avoid calling twice poppedTypes)
Check for the expected return type.
Attachment #581393 -
Attachment is obsolete: true
Attachment #581545 -
Flags: review?(dvander)
Comment on attachment 581545 [details] [diff] [review]
Implement JSOP_LENGTH.
Review of attachment 581545 [details] [diff] [review]:
-----------------------------------------------------------------
Great!
Attachment #581545 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 5•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•