TM: TypeCheck_Promote should be unnecessary




JavaScript Engine
9 years ago
6 years ago


(Reporter: dvander, Unassigned)


Firefox Tracking Flags

(Not tracked)


While investigating bug 521447 I played around with whether or not TypeCheck_Promote is really necessary.  This code dates very far back, before any of the deduceTypeStability() rewrites, and it's complicated, so it'd be nice to understand it fully.

When closing a loop, slots which pass the |isPromoteInt()| test are wrapped in an |f2i()| and written back to the tracker. This is the final point in the integer speculation pipeline. But the tracker automatically writes back promotable integers correctly, so the f2i() shouldn't be necessary. Removing that, we're just setting a tracker value that's already there.

If the slot had no definitions, is there any reason to write it back? Worst case, we're keeping the import load alive and storing back the same value. (This applies in general, I think there's another bug.)

If the slot did have a definition, it would have been set in the tracker. So either it had been written back already, as the correct type, or the store is pending anyway and will be flushed at the loop edge. (Could this create a redundant store?)

To test this, I put in a bunch of relevant asserts and took out TypeCheck_Promote. trace-tests passed and no asserts fired.

The only case I'm not 100% sure about is |TraceRecorder::downRecursion()| since it has weird interactions with the StackFilter. So I'll investigate that before posting a patch.
Summary: TypeCheck_Promote should be unnecessary → TM: TypeCheck_Promote should be unnecessary
Obsolete with the removal of tracejit.
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.