710055 - Merge PushActiveVMFrame and SetVMFrameRegs

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

As far as I can tell, SetVMFrameRegs is always called immediately after PushActiveVMFrame, and only then. So they can be merged. (This also eliminates a state where a StackSegment's regs points to garbage until it gets fixed at the end of EnterMethodJIT, which is only observable from a signal handler.)

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Merge PushActiveVMFrame, SetVMFrameRegs

Patch delayed by fire alarm :)

Comment 2

[Brian Hackett [Laid off!]]

Comment on attachment 581143 [details] [diff] [review]
Merge PushActiveVMFrame, SetVMFrameRegs

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

::: js/src/methodjit/MethodJIT.cpp
@@ +154,5 @@
>  extern "C" void JS_FASTCALL
>  PopActiveVMFrame(VMFrame &f)
>  {
>      f.entryfp->script()->compartment()->jaegerCompartment()->popActiveFrame();
> +    f.cx->stack.repointRegs(f.oldregs);

Can you add back SetVMFrameRegs but make it a no-op?  There are a bunch of other trampolines in js/src/methodjit .asm and .s files for Tier 2 platforms which will be broken by this change, but if SetVMFrameRegs is still there then they will work without needing to be changed.  Alternatively you can fix the trampolines in the other files, but I don't know how straightforward that is (I try to avoid changing the trampolines unless absolutely necessary because of the headache of updating all of them).

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Mer

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Merge PushActiveVMFrame, SetVMFrameRegs

Oh, sorry, that's what I missed. I just grepped for *.h *.cpp. Forgot about *.s and *.asm.

Here's an updated patch carrying over the r+, but with updates for the other architectures. My apologies in advance if I broke any of them, but this particular change seemed pretty straightforward.

(stupid bugzilla/firefox autocomplete UX failure caused me to submit this prematurely)

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/136c73b6457c