Closed Bug 795803 Opened 7 years ago Closed 7 years ago
Monkey: Enable ICing of JSNative and Property Op getters
Branching this out from from bug 786126 to make it easier to track. Enable the generation of IC stubs for calling of PropertyOp getters.
Last try push: https://tbpl.mozilla.org/?tree=Try&rev=96692d56104e
Bug found: bad ordering of checks in TryAttachNativeStub - some checks on the resulting holder object occurred before the null check on the result of lookupProperty (indicating property not found). Fixed, and new try build is: https://tbpl.mozilla.org/?tree=Try&rev=e4bb51cbb3e0 The orange in M(1) is a crash inside GetPcScript, when it tries to walk up the stack frame to get script info and fails. This is because in |buildOOLFakeExitFrame| we use the masm's |framePushed()| value to create the exit frame descriptor. This is supposed to describe the frame size. However, the masm instance used in the OOL stub generator is completely disconnected from the masm instance used to generate the mainline code, which means their |framePushed()| won't be synced. Need to thread the framePushed() into the IC during Ion code generation, and use that to initialize the OOL masm's framePushed().
After fixing the frameSize issue in OOL exit frames, this was the try result: https://tbpl.mozilla.org/?tree=Try&rev=2815b2d60dc1 The error in M(3) was that I was passing the PropertyName directly into the getter without checking for a tinyId/shortId in those cases where those are necessary. Found and fixed that today, new try is at: https://tbpl.mozilla.org/?tree=Try&rev=d57427546d22
Fixed up the 32-bit build issues, rebased to tip, minor changes to ensure that the exit frame from stub code contained a pointer to the stub IonCode so it would be properly rooted, and merged the jsnative and property-op patches into one patch. New try: https://tbpl.mozilla.org/?tree=Try&rev=640ff09a3a11
There was a silly assertion issue introduced in those fixes above which would have led to a lot of try failures. The |pushWithPatch| doesn't update |framePushed| on the macro assembler (as opposed to all the calls to |Push|), which was being checked for consistency with an assert in the code. Changed code to manually adjust |framePushed| For the pushWithPatch. New try is at: https://tbpl.mozilla.org/?tree=Try&rev=32d7866c00f2
Changed title to reflect the fact that the patch supports both JSNative and PropertyOp getters.
Summary: IonMonkey: Enable ICing of PropertyOp getters → IonMonkey: Enable ICing of JSNative and PropertyOp getters
The oranges in the previous try are from the tip I was working off of, and relate to memory leaks that are unrelated to my patch (checked on tbpl). Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/fa3d21b84a63 See bug 786126 for reviews. Jan reviewed two separate patches which I've consolidated into one for the push since they sit right on top of each other in the code.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.