IonMonkey: Minor cleanups around polymorphic inlining

RESOLVED FIXED in mozilla21

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: sstangl, Unassigned)

Tracking

unspecified
mozilla21
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Created attachment 712054 [details] [diff] [review]
patch

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)
(Reporter)

Comment 1

6 years ago
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+
(Reporter)

Comment 3

6 years ago
Thanks! The spew was moved into trimToAndMaybePatchTargets().

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd80e0af25cd

Comment 4

6 years ago
https://hg.mozilla.org/mozilla-central/rev/cd80e0af25cd
Status: NEW → RESOLVED
Last Resolved: 6 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.