Closed Bug 844246 Opened 12 years ago Closed 12 years ago

BaselineCompiler: Run inference in time to ensure that Ion can inline calls

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

Details

Attachments

(2 files)

Right now, Ion will only inline a scripted call if TI has run on the called script; analyzing types in the script during IonBuilder can change type sets which have previously been used by the builder, so IonBuilder (well, TypeInferenceOracle::canEnterInlinedFunction) simply checks and skips callees that TI hasn't analyzed. This works fine with JM+TI as compiling with JM+TI requires running inference on a script, but plays havoc with Baseline --- if the caller has a higher use count than the callee (which will very often be the case) then the callee will not be inlined unless some future type change invalidates the caller but not the callee. Baseline needs a mechanism to analyze the types in callees at the start of IonBuilder, if they are hot enough that they could be inlined. This can go away after bug 804676.
Assignee: general → bhackett1024
I ran into this when looking at deltablue. The following patches recursively does ensureRanInference on callees snooped from baseline IC caches before entering into Ion compilation.
Attached patch alternative (deleted) — Splinter Review
This is a slightly different patch that does a couple things differently: - No state is kept in the baseline structures, so that scripts can be scanned multiple times during different compilations (this is good to do as the callees' use counts can change and thus the behavior of scanning the caller). - Uses TI state, so that types can be analyzed in 'this' values for foo.call and foo.apply invocations, allowing those to be inlined by Ion. This improves my v8bench score by 400 points or so and also improves SS a few ms.
Attachment #717330 - Flags: review?(kvijayan)
Comment on attachment 717330 [details] [diff] [review] alternative Review of attachment 717330 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: js/src/ion/TypeOracle.cpp @@ +96,5 @@ > + if (JSObject *singleton = calleeTypes->getSingleObject(i)) { > + if (singleton->isFunction() && singleton->toFunction()->hasScript()) > + script = singleton->toFunction()->nonLazyScript(); > + else > + continue; Style nit: The code structure is a bit hard to read as the first branch "falls through", but the continue comes after the fallthrough, but is on the other branch so the fallthrough doesn't actually hit it. Would be clearer as: if (!singleton->isFunction()) continue; if (!singleton->toFunction()->hasScript()) continue; script = singleton->toFunction()->nonLazyScript(); @@ +102,5 @@ > + JSFunction *fun = type->interpretedFunction; > + if (fun && fun->hasScript()) > + script = fun->nonLazyScript(); > + else > + continue; See comment above. if (!fun || !fun->hasScript()) continue; script = fun->nonLazyScript(); @@ +104,5 @@ > + script = fun->nonLazyScript(); > + else > + continue; > + } else { > + continue; Is it possible for an object to be something other than a Singleton or a TypeObject? Should there be a JS_NOT_REACHED here?
Attachment #717330 - Flags: review?(kvijayan) → review+
TypeSets can return NULL for both getSingleObject and getTypeObject, as for larger sets they are iterating over a hashtable. https://hg.mozilla.org/projects/ionmonkey/rev/8245e95ef08f
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: