Closed
Bug 1169214
Opened 9 years ago
Closed 9 years ago
[MBaselineCache] Adjust JitStubHelpers to have ionmonkey variants
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: h4writer, Assigned: h4writer)
References
Details
Attachments
(4 files, 1 obsolete file)
38.30 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
3.52 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
3.69 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
4.85 KB,
patch
|
h4writer
:
review+
|
Details | Diff | Splinter Review |
JitStubHelper has callVM/TailCallVM/EnterExitFrame/... helpers. Those are currently specialised for Baseline. We need similar function for IonMonkey.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → hv1989
Attachment #8621587 -
Flags: review?(jdemooij)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8621588 -
Flags: review?(jdemooij)
Assignee | ||
Comment 3•9 years ago
|
||
Simplified the non-tail calls a bit.
Attachment #8621588 -
Attachment is obsolete: true
Attachment #8621588 -
Flags: review?(jdemooij)
Attachment #8621606 -
Flags: review?(jdemooij)
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8621608 -
Flags: review?(jdemooij)
Comment 5•9 years ago
|
||
Comment on attachment 8621587 [details] [diff] [review] Part 1: Create Ion shared stub helpers Review of attachment 8621587 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/SharedIC.cpp @@ +740,5 @@ > > void > ICStubCompiler::enterStubFrame(MacroAssembler& masm, Register scratch) > { > + if (engine_ == Engine::Baseline) { Nit: no {} @@ +760,5 @@ > { > MOZ_ASSERT(entersStubFrame_ && inStubFrame_); > inStubFrame_ = false; > + > + if (engine_ == Engine::Baseline) { And here.
Attachment #8621587 -
Flags: review?(jdemooij) → review+
Comment 6•9 years ago
|
||
Comment on attachment 8621606 [details] [diff] [review] Part 2: Add the x86 shared stub helpers Review of attachment 8621606 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/x86/SharedICHelpers-x86.h @@ +129,5 @@ > inline void > EmitIonCallVM(JitCode* target, size_t stackSlots, MacroAssembler& masm) > { > + // FIXME: #1169214: Make sure all stubcode use push/pop calls that > + // adjust the frame pushed state. Nit: I think we can remove the comment; if a stub does not update the frame pushed state it's a bad crash/bug, not just an optional FIXME. @@ +136,5 @@ > + masm.call(target); > + > + // Remove rest of the frame left on the stack. We remove the return address > + // which is implicitly poped when returning. > + int framePop = sizeof(ExitFrameLayout) - sizeof(void*); Nit: s/int/size_t/
Attachment #8621606 -
Flags: review?(jdemooij) → review+
Comment 7•9 years ago
|
||
Comment on attachment 8621608 [details] [diff] [review] Part 3: Add the x64 shared stub helpers Review of attachment 8621608 [details] [diff] [review]: ----------------------------------------------------------------- See nits for the previous patch :)
Attachment #8621608 -
Flags: review?(jdemooij) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e93437f8667 https://hg.mozilla.org/integration/mozilla-inbound/rev/39305ec41209 https://hg.mozilla.org/integration/mozilla-inbound/rev/590916330cee
Comment 9•9 years ago
|
||
This patchset forgot about (and consequently broke) ARM64!
Comment 10•9 years ago
|
||
Follow-up patch to fix ARM64 build.
Attachment #8651240 -
Flags: review?(hv1989)
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2e93437f8667 https://hg.mozilla.org/mozilla-central/rev/39305ec41209 https://hg.mozilla.org/mozilla-central/rev/590916330cee
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8651240 [details] [diff] [review] Follow-up to port the patch to ARM64. Review of attachment 8651240 [details] [diff] [review]: ----------------------------------------------------------------- Don't we have treeherder arm64 builds? (even if only builds and no tests?). It took me a while to get it green. So I was happy it was finally green and as a result I pushed. In order to not have this anymore. Do we have simulator so I can build and test locally?
Attachment #8651240 -
Flags: review?(hv1989) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•