Closed Bug 601657 Opened 14 years ago Closed 14 years ago

JM: Remove FrameState::syncData() hack

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: sstangl)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

FrameState::syncData() has a hack where it syncs types if the payload was constant. Every place that uses syncData() has to then do a constant-check and avoid syncType(). The order of data/type syncing is also implicitly mandated.

No one seems to really know why we do this. Sean suspects something to do with double constants, since historically we've treated their type differently. It also might have to do with earlier unsound optimizations to avoid syncing the payload of |undefined|.

The latter no longer exists. For the former, we should audit the use of getKnownType in FrameState with regards to syncing, and get rid of this hack.
Assignee: general → sstangl
Redefine getPayload(), getTypeTag() to be shared between 32-bit and 64-bit. Redefine syncData() and syncType() to eliminate the constant-saving hack in syncData(). Refactor some code that can be expressed more simply as a result.
Attachment #480845 - Flags: review?(dvander)
Comment on attachment 480845 [details] [diff] [review]
Eliminate syncData() constant hack.

Yay!

>-        masm.move(Imm64(known->getKnownShiftedTag()), maskReg);
>+        masm.move(Imm64(known->getKnownTag()), maskReg);

Should this be ImmType, for clarity?

BTW, the reason for the mysterious "if (closed) .. forgetEntry" is that forgetEntry makes sure to remove any copies and free associated registers. resetSynced alone won't do that.
Attachment #480845 - Flags: review?(dvander) → review+
ImmTag, yeah. Usage of ImmTag is very much discouraged, as it opens boxes into the implementation of the type system -- but we're doing that anyway in that location, so might as well.

Also fixed a register clobber bug for x64 in jsop_stricteq(). Great.

http://hg.mozilla.org/tracemonkey/rev/c3bd2594777a
Whiteboard: fixed-in-tracemonkey
There was no clobber bug there, frame.allocReg() isn't pinned to an FE so it is impossible to evict. I'm surprised this change didn't assert trying to pin an owned register.
http://hg.mozilla.org/mozilla-central/rev/c3bd2594777a
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.