Closed Bug 820406 Opened 8 years ago Closed 8 years ago

BaselineCompiler: Store the scope chain

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(1 file, 1 obsolete file)

Need this to compile DEFVAR, NAME etc.
Depends on: 820816
Attached patch Patch (obsolete) — Splinter Review
I tried to handle heavyweight functions too, but we have to compile more ops to test it. So this just aborts on heavyweight functions until we support DEFVAR, NAME, LAMBDA etc.
Attachment #691364 - Flags: review?(kvijayan)
Attachment #691364 - Flags: review?(kvijayan)
Attached patch PatchSplinter Review
Attachment #691364 - Attachment is obsolete: true
Attachment #691368 - Flags: review?(kvijayan)
Comment on attachment 691368 [details] [diff] [review]
Patch

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

Thanks for taking care of this.  Unblocks some important ops for implementation.

::: js/src/ion/arm/IonFrames-arm.h
@@ +462,5 @@
>      uint32_t loScratchValue;
>      uint32_t hiScratchValue;
>      size_t frameSize;
> +    JSObject *scopeChain;
> +    uint32_t dummy; // Keep frame 8-byte aligned.

I don't understand the need for this.  Without it, BaselineFrame is 16 bytes on ARM.  This bumps it to 20 bytes.  Are we expecting the stack to be at a 4-byte offset when this is pushed?
Attachment #691368 - Flags: review?(kvijayan) → review+
(In reply to Kannan Vijayan [:djvj] from comment #3)
>
> I don't understand the need for this.  Without it, BaselineFrame is 16 bytes
> on ARM.  This bumps it to 20 bytes.  Are we expecting the stack to be at a
> 4-byte offset when this is pushed?

Yeah, the 4-byte offset is for the old frame pointer we push on the stack. See the BaselineFrame static assert, it asserts (sizeof(BaselineFrame) + FramePointerOffset) % 8 == 0.
https://hg.mozilla.org/projects/ionmonkey/rev/bc21eceb1c25
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.