Closed
Bug 1003825
Opened 10 years ago
Closed 10 years ago
Get funapply tests working again
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: h4writer, Assigned: h4writer)
References
Details
Attachments
(1 file, 2 obsolete files)
9.20 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
Just making sure the tests test again for which they were added. They didn't fail anymore when disabling the baselineBailouts hacks.
Assignee: nobody → hv1989
Attachment #8415283 -
Flags: review?(jdemooij)
Assignee | ||
Comment 2•10 years ago
|
||
Forgot to qref
Attachment #8415283 -
Attachment is obsolete: true
Attachment #8415283 -
Flags: review?(jdemooij)
Attachment #8415285 -
Flags: review?(jdemooij)
Comment 3•10 years ago
|
||
Comment on attachment 8415285 [details] [diff] [review] get-funapply-test-working Review of attachment 8415285 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/IonBuilder.cpp @@ +4978,5 @@ > + JSFunction *target = getSingleCallTarget(calleeTypes); > + > + // Try optimizing fun.apply(foo, arguments). > + do { > + if (info().executionMode() != DefinitePropertiesAnalysis) Should be ==, but then we'll no longer inline for the definite properties analysis, see this comment in jsop_funapplyarguments: // We also try this path when doing the definite properties analysis, as we // can inline the apply() target and don't care about the actual arguments // that were passed in. @@ +4998,5 @@ > + if (!target) > + break; > + > + if (!target->isNative() || target->native() != js_fun_apply) > + break; If we have: fun.apply(foo, arguments); And fun.apply is not js_fun_apply, we'll now emit a call with MIRType_MagicOptimizedArguments as argument. js_fun_apply knows what to do with MagicValue(JS_OPTIMIZED_ARGUMENTS), but this will fail for other callees.
Attachment #8415285 -
Flags: review?(jdemooij)
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8415285 -
Attachment is obsolete: true
Attachment #8416488 -
Flags: review?(jdemooij)
Comment 5•10 years ago
|
||
Comment on attachment 8416488 [details] [diff] [review] Patch: without the funapply changes Review of attachment 8416488 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/IonBuilder.cpp @@ +642,5 @@ > IonSpew(IonSpew_Scripts, "Recompiling script %s:%d (%p) (usecount=%d, level=%s)", > script()->filename(), script()->lineno(), (void *)script(), > (int)script()->getUseCount(), OptimizationLevelString(optimizationInfo().level())); > } else { > + IonSpew(IonSpew_Scripts, "Compiling script %s:%d (%p) (usecount=%d, level=%s)", Nice to have this fixed. ::: js/src/jsinfer.h @@ +156,5 @@ > * when it is not known whether a lazy arguments value can be used. > */ > ArgumentsUsageAnalysis > }; > +inline const char * Nit: add an empty line before this line.
Attachment #8416488 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #5) > ::: js/src/jsinfer.h > @@ +156,5 @@ > > * when it is not known whether a lazy arguments value can be used. > > */ > > ArgumentsUsageAnalysis > > }; > > +inline const char * > > Nit: add an empty line before this line. Strangely this had already a newline in my original patch https://hg.mozilla.org/integration/mozilla-inbound/rev/079264ecb678
https://hg.mozilla.org/mozilla-central/rev/079264ecb678
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•