931487 - Backtracking allocator: avoid spilling two different register groups to the same argument slot

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Dan Gohman [:sunfish]]

Attachment: regalloc-dont-share-arg-slots.patch

When the backtracking allocator creates a register group containing a register copied from an incoming argument slot, it assigns that argument slot to be the canonical spill location for the group. However, this breaks when there is an OSR entry with the same argument slot copied into a virtual register which is not in the same group. Both groups attempt to use the argument slot as a spill slot for their entire extent, and they end up trampling each other.

This can be seen in auto-regress/bug700295.js, for example.

The attached patch fixes this by limiting this form of canonical spill slot assignment to regular stack slots, excluding argument slots. This is somewhat conservative, because it excludes the use of argument slots as spill slots even in cases where they would be safe, but it is a simple fix.

Comment 1

[Brian Hackett [Laid off!]]

Comment on attachment 822915 [details] [diff] [review]
regalloc-dont-share-arg-slots.patch

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

I don't think that any vregs will have a preset stack slot allocation, so the only real point of this block is to allow vregs in the same group as the argument (and the argument itself) to be spilled to the argument slot.  Would it work to just not use the canonical spill slot for the OSR entry's group, if its group's lifetime is different from and overlaps that of the original argument's group?

Comment 2

[Dan Gohman [:sunfish]]

No, it doesn't appear sufficient to just disable the canonical spill slot for the OSR entry's group. If the normal entry's group contains other non-argument registers that happen to be defined in the OSR entry block, the group can still spill to the argument slot in the OSR block and clobber it while it's live for other purposes.

It might be sufficient to disable use of argument spill slots for register groups that have any registers defined in the OSR entry block, but I haven't tried that yet.

It's also worth noting that the patch proposed for bug 906858 also fixes/worksaround this bug.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c610c3ac1ba9