Closed Bug 810946 Opened 12 years ago Closed 12 years ago

BaselineCompiler: Mark baseline frames

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached patch Patch (obsolete) (deleted) — Splinter Review
Stores the frame size without VMFunction arguments in the frame, then uses that to mark all locals and stack values.
Attachment #680631 - Flags: review?(kvijayan)
Attached patch Patch v2 (deleted) — Splinter Review
Fix a typo.
Attachment #680631 - Attachment is obsolete: true
Attachment #680631 - Flags: review?(kvijayan)
Attachment #680633 - Flags: review?(kvijayan)
Comment on attachment 680633 [details] [diff] [review] Patch v2 Review of attachment 680633 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/shared/IonFrames-x86-shared.h @@ +450,5 @@ > + static const uint32_t FramePointerOffset = sizeof(void *); > + > + static inline size_t offsetOfFrameSize() { > + return -offsetof(BaselineFrame, frameSize) - sizeof(uint32_t); > + } These offsets are intended to be used with reference to a pointer immediately above the BaselineFrame. At least that's what my reading of the marking code seems to indicate. This expression may be clearer as: -(sizeof(BaselineFrame) - offsetOf(BaselineFrame, frameSize)) And maybe we should call these method backwardsOffsetOf... or reverseOffsetOf... ? The offsetOf naming convention seems pretty solidly consistent to refer to the offset of things from a pointer to the start of a structure, not the end. @@ +456,5 @@ > + return -(sizeof(BaselineFrame) + index * sizeof(js::Value)) - sizeof(js::Value); > + } > + static inline size_t offsetOfArg(size_t index) { > + return FramePointerOffset + js::ion::IonJSFrameLayout::offsetOfActualArg(index); > + } The above statement about renaming the offsetOf... methods wouldn't apply to offsetOfArg since it does compute a forward offset.
Attachment #680633 - Flags: review?(kvijayan) → review+
Attached patch Dump baseline frames (deleted) — Splinter Review
Attachment #681014 - Flags: review?(nicolas.b.pierron)
Pushed with the reverse* names + a comment. https://hg.mozilla.org/projects/ionmonkey/rev/7c9cfaafa4a1
Comment on attachment 681014 [details] [diff] [review] Dump baseline frames Review of attachment 681014 [details] [diff] [review]: ----------------------------------------------------------------- Sounds good. I am happy to see that adding the dumpBaseline function help clarify the bunch of JS_ASSERT of the Mark function.
Attachment #681014 - Flags: review?(nicolas.b.pierron) → review+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: