Skip to content

Fix two critical Cmm register bugs

sheaf requested to merge sheaf/ghc:T26542 into master

This MR fixes #26550 (closed) and #26542 (closed).

First commit (fixing #26550 (closed)): computing conflicting local registers in Cmm sink

This fixes a serious oversight in the code of GHC.Cmm.Sink.conflicts which computes which local registers conflict between an assignment and a Cmm statement.

If we have:

  assignment: <local_reg> = <expr>
        node: <local_reg> = <other_expr>

then clearly the two conflict, because we cannot move one statement past the other, as they assign two different values to the same local register. (Recall that conflicts (local_reg,expr) node is False if and only if the assignment local_reg = expr can be safely commuted past statement node.)

The fix is to update GHC.Cmm.Sink.localRegistersConflict to take into account when both:

  1. node defines the LHS local register of the assignment,
  2. node defines a local register used in the RHS expression of the assignment.

The bug is precisely that we were previously missing condition (1).

Second commit (fixing #26542 (closed)): update allocated register format in allocRegsAndSpill_spill

Make sure to correctly update the format in GHC.CmmToAsm.Reg.Linear.allocRegsAndSpill_spill.

When we are allocating a virtual register to a real register and we decide to spill the data currently residing in a register, we need to:

  1. Spill the data currently in the register. That is, do a spill with a format that matches what's currently in the register.
  2. Update the register assignment, allocating a virtual register to this real register, but crucially updating the format of this assignment.

Due to shadowing in the Haskell code for allocRegsAndSpill_spill, we were mistakenly re-using the old format. This could lead to a situation where:

  1. We were using xmm6 to store a Double#.
  2. We want to store a DoubleX2# into xmm6, so we spill the current contents of xmm6 to the stack using a scalar move (correct).
  3. We update the register assignment, but we fail to update the format of the assignment, so we continue to think that xmm6 stores a Double# and not a DoubleX2#.
  4. Later on, we need to spill xmm6 because it is getting clobbered by another instruction. We then decide to only spill the lower 64 bits of the register, because we still think that xmm6 only stores a Double# and not a DoubleX2#.

By making sure we update the assignment to reflect what's currently in the register, we avoid this bug.

Edited by sheaf

Merge request reports

Loading