1169213 - [MBaselineCache] Add frame type for an IonStub

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Comment 1

[Hannes Verschore [:h4writer]]

Attachment: bug1169213-jitframe-ionstub

This introduces JitFrame_IonStub. This will make it possible to do vmcalls out of sharedstubs (baseline stubs that work also in ionmonkey).

Comment 2

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8612288 [details] [diff] [review]
bug1169213-jitframe-ionstub

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

Ask me for review on the code which is using the JitFrame_IonStub.

::: js/src/jit/JitFrames.cpp
@@ +1540,5 @@
>            case JitFrame_IonJS:
>              MarkIonJSFrame(trc, frames);
>              break;
> +          case JitFrame_IonStub:
> +            MarkIonStubFrame(trc, frames);

Rename MarkIonStubFrame to MarkJitStubFrame and use it for both baseline and Ion stub frames.

::: js/src/jit/JitFrames.h
@@ +893,5 @@
>  }
>  
>  class ICStub;
>  
> +class IonStubFrameLayout : public CommonFrameLayout

JitStubFrameLayout?

@@ +898,4 @@
>  {
> +    // Info on the stack
> +    // Note: The stack architecture grows down. Thus sequence in stack is
> +    // in the opposite order as class members.

The sequence of data in the stack is the same as in the class, otherwise reinterpret_cast of stack pointers would never work.
What you probably meant is the content has to be "pushed to" the stack in opposite order, as the stack pointer grows down when we push, but this is not specific to this class.

Thus, I think this comment does not bring any value and I recommend to remove it.

@@ +906,5 @@
> +    // | - Descriptor     | => Marks end of JitFrame_IonJS
> +    // | - returnaddres   |
> +    // +------------------+
> +    // | - StubPtr        | => First thing pushed in a stub only when the stub will do
> +    // --------------------    a vmcall. Else we cannot have IonStubFrame.

I guess this is inherited from legacy code, but why StubPtr is not a member of this class?
Maybe Kannan or Jandem can answer?

@@ +927,5 @@
> +class BaselineStubFrameLayout : public IonStubFrameLayout
> +{
> +    // Info on the stack
> +    // Note: The stack architecture grows down. Thus sequence in stack is
> +    // in the opposite order as class members.

Remove this part of the comment.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/63b1e449beda

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/63b1e449beda