Closed Bug 616744 Opened 11 years ago Closed 11 years ago
JM: getelement pic for arguments
After my first contact with the PIC, i got the idea to do something harder. So i eventually figured, we would always take a slow path for arguments objects. Believe me i wasn't disappointed with the amount of thinking, i had to built this. Happily the jit brought massive speedup.
I am still working on this, trying to optimize and reduce code size. Some things are also x32 specific.
The liveArguments path is totally buggy, btw.
Once you're done with this, I'd love to see what sort of numbers you get on http://dromaeo.com/?prototype before and after the patch!
So I got this working with arguments with live arguments (no pointer to FrameEntry), and arguments < the number of formal arguments of the function. So the only thing left are the overflow arguments. I think i would need one more register, feedback appreciated.
11 years ago
This approach seems good - normally when there is lots of internal control flow, I'd prefer making separate ICs (one for each major case), but here the cases are pretty small and it seems similar to what we do for call objects. But just to reduce the function length you may want to separate it into separate functions. Grabbing extra registers in ICs is tricky. I think you can get away with two if you use the stack to save before you clobber. If not, you could take a snapshot of the register state in the compiler, and save that in the IC. That way you know what's available and what might need to be saved (typed arrays do this, bug 594247).
OS: Windows XP → All
So got the basic thing working. Just need to convert some statics to offset use, and possibly some comments? I used push/pop to solve my register problem. +Feedback and i will finish this.
So this basically the same as WIP 2. I only made it non platform depend and added one test.
Sorry forgot to hg add the test.
Attachment #536121 - Flags: review?(dvander)
Comment on attachment 536121 [details] [diff] [review] v1 with the test Review of attachment 536121 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, r=me with one comment: ::: js/src/methodjit/PolyIC.cpp @@ +2358,5 @@ > + > + Address privateData(typeReg, offsetof(JSObject, privateData)); > + Jump liveArguments = masm.branchPtr(Assembler::NotEqual, privateData, ImmPtr(0)); > + > + masm.loadPayload(Address(typeReg, JSObject::getFixedSlotOffset(ArgumentsObject::DATA_SLOT)), objReg); This must be loadPrivate()
Attachment #536121 - Flags: review?(dvander) → review+
Please add a commit comment and from/user line; it's really hard to push the patch otherwise...
(In reply to comment #12) > Please add a commit comment and from/user line; it's really hard to push the > patch otherwise... Sorry, i didn't think of that as i am used to commit patches by myself. Because i have the rights now i just did it :) http://hg.mozilla.org/integration/mozilla-inbound/rev/da50621162f3
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.