Closed
Bug 1383777
Opened 7 years ago
Closed 7 years ago
Support additional idempotent ICs
Categories
(Core :: JavaScript Engine: JIT, enhancement)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: bhackett1024, Assigned: bhackett1024)
References
Details
Attachments
(1 file)
24.19 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
GetPropIRGenerator::tryAttachIdempotentStub() has this comment in it: // For idempotent ICs, only attach stubs for plain data properties. // This ensures (1) the lookup has no side-effects and (2) Ion has complete // static type information and we don't have to monitor the result. Because // of (2), we don't support for instance missing properties or array // lengths, as TI does not account for these cases. On Speedometer v2, accessing missing properties and array lengths on idempotent ICs account for 1444 invalidations, or 69% of the total number of idempotent IC invalidations. These property accesses do not have side effects and can be performed in an idempotent fashion. (2) above is not a fundamental restriction, static TI information isn't necessary as long as the ICs produce values which the MIR node's consumers are designed to handle.
Assignee | ||
Comment 1•7 years ago
|
||
This patch keeps track of which idempotent caches may produce undefined or int32 values, allowing missing property and object length ICs to be attached. This produces the expected reduction in invalidations on speedometer.
Assignee: nobody → bhackett1024
Attachment #8889554 -
Flags: review?(jdemooij)
Comment 2•7 years ago
|
||
Comment on attachment 8889554 [details] [diff] [review] patch Review of attachment 8889554 [details] [diff] [review]: ----------------------------------------------------------------- Sweet. ::: js/src/jit/CacheIR.h @@ +1141,5 @@ > > enum class CanAttachGetter { Yes, No }; > > +// Flags used to describe what values a GetProperty cache may produce. > +enum GetPropertyResultFlags { enum class The patch already uses the GetPropertyResultFlags:: prefix so it should just compile hopefully. ::: js/src/jit/IonIC.cpp @@ +140,5 @@ > jsbytecode* pc = ic->idempotent() ? nullptr : ic->pc(); > bool isTemporarilyUnoptimizable = false; > GetPropIRGenerator gen(cx, outerScript, pc, ic->kind(), ic->state().mode(), > + &isTemporarilyUnoptimizable, val, idVal, val, canAttachGetter, > + ic->resultFlags()); canAttachGetter is defined like this: CanAttachGetter canAttachGetter = ic->monitoredResult() ? CanAttachGetter::Yes : CanAttachGetter::No; So we could remove CanAttachGetter and replace its uses in CacheIR.cpp with the resultFlags & ...Monitored check, to simplify the GetProp code.
Attachment #8889554 -
Flags: review?(jdemooij) → review+
Pushed by bhackett@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/08bef58cdb9d Support idempotent ICs that access missing properties and object lengths, r=jandem.
Comment 4•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/08bef58cdb9d
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•