Closed Bug 839727 Opened 9 years ago Closed 9 years ago

IonMonkey: Minor cleanups around polymorphic inlining

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: sstangl, Unassigned)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Splitting up the changes to the polymorphic inlining code into a number of bugs, so that the final review isn't too crazy, and so that I don't have rebase hell along the way. Hopefully, it's even easier to review!

This patch fixes a bunch of stylistic nits without affecting logic:

1) Cleans up getInlineableGetPropertyCache().
2) Uses CallInfo in makeInliningDecision().
3) Moves InlinePropertyTable::trimToAndMaybePatchTargets() out of headers.
4) Writes patchInlinedReturns() in terms of a helper function for clarity.
5) Cleans up CodeGenerator::visitPolyInlineDispatch().
6) Fixes various nits in MIR.h.
Attachment #712054 - Flags: review?(hv1989)
Comment on attachment 712054 [details] [diff] [review]
patch

Hannes is away for a bit -- flipping review.
Attachment #712054 - Flags: review?(hv1989) → review?(kvijayan)
Comment on attachment 712054 [details] [diff] [review]
patch

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

Cool.  Refactors and cleanups look pretty good.  One concern I have is that the inlining spew seems to have been removed along with that.  It'd be nice to have something which indicates that the optimization is being performed - as it represents some reasonably major abstract stack surgery... and for perf tracking purposes.
Attachment #712054 - Flags: review?(kvijayan) → review+
Thanks! The spew was moved into trimToAndMaybePatchTargets().

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd80e0af25cd
https://hg.mozilla.org/mozilla-central/rev/cd80e0af25cd
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Nice work Sean!
You need to log in before you can comment on or make changes to this bug.