1168753 - [MBaselineCache] Make VMFunction aware of BaselineFrame

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Hannes Verschore [:h4writer]]

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

Comment 1

[Hannes Verschore [:h4writer]]

Attachment: Part 1: WIP:

Comment 2

[Hannes Verschore [:h4writer]]

Attachment: 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.

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: 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.

Comment 4

[Jan de Mooij [:jandem]]

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?

Comment 5

[Hannes Verschore [:h4writer]]

Attachment: 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).

Comment 6

[Jan de Mooij [:jandem]]

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.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c4f04873418

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0c4f04873418