Closed
Bug 562729
Opened 14 years ago
Closed 14 years ago
JM: statically bind global functions where possible
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dvander, Assigned: dvander)
References
Details
Attachments
(1 file, 1 obsolete file)
10.86 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
Same idea as bug 561923, but separating out for sanity. This code: function f() { } f(); print(f); This results in "CALLNAME" and "NAME". We would like this to be CALLGLOBAL ; GETGLOBAL or at the very least the unbound variants that are easy to inline cache.
Assignee | ||
Updated•14 years ago
|
Assignee: general → dvander
Assignee | ||
Comment 1•14 years ago
|
||
passes js & reftests
Assignee | ||
Comment 2•14 years ago
|
||
Early push to JM: http://hg.mozilla.org/users/danderson_mozilla.com/jaegermonkey/rev/5a9a35350982
Assignee | ||
Comment 3•14 years ago
|
||
Attachment #444473 -
Attachment is obsolete: true
Attachment #445169 -
Flags: review?(brendan)
Comment 4•14 years ago
|
||
Comment on attachment 445169 [details] [diff] [review] rebased >+ /* >+ * Previously this would be JSOP_NAME in some cases, which would push a >+ * global object (see bug 555246 for more details). Now we're pushing >+ * NULL instead, which creates more trace instability. As a quick hack >+ * until bug 556277 fixes all |this| problems, do a best-case test to see >+ * if we can push a global object. Use the FIXME: bug 556277 convention here and in jstracer.cpp for the parallel code, and please do make sure this all goes away. >+ if (JSScopeProperty *sprop = scope->lookup(ATOM_TO_JSID(atom))) { >+ /* >+ * If the property was found, bind the slot immediately if >+ * we can. If we can't, don't bother emitting a GVAR op, >+ * since it's unlikely that it will optimize either. >+ */ >+ uint32 index; >+ if (!sprop->configurable() && >+ SPROP_HAS_VALID_SLOT(sprop, globalObj->scope()) && >+ sprop->hasDefaultGetterOrIsMethod() && >+ sprop->hasDefaultSetter() && >+ pn->pn_type != TOK_FUNCTION && >+ cg->addGlobalUse(atom, sprop->slot, &index) && Per previous review on ContextAllocPolicy (needed elsewhere for OOM reporting), don't include this in the && chain, instead use an if in the braced then below, and fail immediately if addGlobalUse fails. >+ index != FREE_UPVAR_COOKIE) But fail soft if index == FREE_UPVAR_COOKIE. >+ { >+ pn->pn_cookie = index; >+ pn->pn_dflags |= PND_GVAR | PND_BOUND; >+ pn->pn_op = JSOP_GETGLOBAL; >+ } >+ >+ JS_UNLOCK_SCOPE(cx, scope); Don't forget to unlock before failing immediately on OOM. >@@ -2989,6 +3086,12 @@ Parser::functionDef(uintN lambda, bool n > pn->pn_body = body; > } > >+ if (!outertc->inFunction() && topLevel && funAtom && !lambda && outertc->compiling()) { Nit: outertc->compiling() && !outertc->inFunction(), reads a bit better. Wonder if gcc can optimize into one mask and test? /be
Attachment #445169 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 5•14 years ago
|
||
This landed on JM a long time ago.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•