522189 - TM: TypeCheck_Promote should be unnecessary

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Obsolete with the removal of tracejit.