[MBaselineCache] Make VMFunction aware of BaselineFrame

RESOLVED FIXED in Firefox 41

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: h4writer, Assigned: h4writer)

Tracking

unspecified
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

3 years ago
Make it possible to ask if a BaselineFrame needs to get pushed and let the callVM/tailCallVM call take care of pushing them. Makes the code more uniform between in shared (ion/baseline) stubs
(Assignee)

Updated

3 years ago
Blocks: 1161516
(Assignee)

Updated

3 years ago
Depends on: 1125481
(Assignee)

Comment 1

3 years ago
Created attachment 8612241 [details]
Part 1: WIP:
(Assignee)

Comment 2

3 years ago
Created attachment 8612243 [details] [diff] [review]
Part 2: Move the pushBaselineFramePtr to the helper function

Part 1 makes it possible to ask a VMFunction needs a BaselineFrame*. This function is to move the actual pushing to the helper function. This is because ionmonkey doesn't need a BaselineFrame* and will push 0 in that case. This simplification is to make sure that the generateCode functions don't need to worry for which engine they are compiling.
Attachment #8612243 - Flags: review?(jdemooij)
(Assignee)

Updated

3 years ago
Assignee: nobody → hv1989
(Assignee)

Comment 3

3 years ago
Created attachment 8612256 [details] [diff] [review]
Part 2: Move the pushBaselineFramePtr to the helper function

I saw I forgot one place where we did (ICStub*, BaselineFrame*). Not a problem, except for consistency.
Attachment #8612243 - Attachment is obsolete: true
Attachment #8612243 - Flags: review?(jdemooij)
Attachment #8612256 - Flags: review?(jdemooij)
Comment on attachment 8612256 [details] [diff] [review]
Part 2: Move the pushBaselineFramePtr to the helper function

Review of attachment 8612256 [details] [diff] [review]:
-----------------------------------------------------------------

Clever idea, but can't we get the same effect by adding a pushBaselineFramePtr() method to ICStubCompiler, and then doing |if (engine == foo) ..| there? Or a PushBaselineFramePtr(masm, engine) in SharedICHelpers.h?
Attachment #8612256 - Flags: review?(jdemooij)
(Assignee)

Comment 5

3 years ago
Created attachment 8615368 [details] [diff] [review]
Push frame ptr

This would do the same, without being smart.

(Though I think it was better before. The fact that callVM/tailCallVM pushed the frameptr, made it possible to know if we did an enterStubFrame or not. Now I had to add an extra variable to track that).
Attachment #8612241 - Attachment is obsolete: true
Attachment #8612256 - Attachment is obsolete: true
Attachment #8615368 - Flags: review?(jdemooij)
Comment on attachment 8615368 [details] [diff] [review]
Push frame ptr

Review of attachment 8615368 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

::: js/src/jit/BaselineIC.cpp
@@ +267,5 @@
>          // Push stub pointer.
>          masm.push(ICStubReg);
>  
> +        // Push JitFrameLayout pointer.
> +        pushFramePtr(masm, R0.scratchReg());

Nit: please remove the comment while you're here. I think the argument was changed from JitFrmaeLayout* to BaselineFrame* at some point.
Attachment #8615368 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/0c4f04873418
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.