Closed Bug 680432 Opened 10 years ago Closed 10 years ago

IonMonkey: Greedy Allocator: erasure of xmm register.

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: nbp, Assigned: dvander)

References

Details

Attachments

(3 files, 2 obsolete files)

The attached test highlight the issue.  At the end, the first block has the following set of instructions:

    begin_LIR
      1 parameter ([x (arg:0)]) <|@
      2 parameter ([x (arg:8)]) <|@
      3 double ([f (=xmm14)]) <|@
      0 movegroup ()[=xmm14 -> stack:d5] <|@
      4 double ([f (=xmm13)]) <|@
      5 double ([f (=xmm11)]) <|@
      6 double ([f (=xmm10)]) <|@
      7 double ([f (=xmm9)]) <|@
      8 double ([f (=xmm8)]) <|@
      9 double ([f (=xmm7)]) <|@
      10 double ([f (=xmm6)]) <|@
      11 double ([f (=xmm5)]) <|@
      12 double ([f (=xmm4)]) <|@
      13 double ([f (=xmm3)]) <|@
      14 double ([f (=xmm2)]) <|@
      15 double ([f (=xmm1)]) <|@
      16 double ([f (=xmm0)]) <|@
      17 double ([f (=xmm12)]) <|@
      18 double ([f (=xmm14)]) <|@
      0 movegroup ()[=xmm14 -> stack:d3] <|@
*     19 double ([f (=xmm14)]) <|@
*     0 movegroup ()[stack:d5 -> =xmm14] <|@
      20 value ([x (=r15)]) <|@
      0 movegroup ()
         [=xmm14 -> stack:d4],
         [=xmm13 -> stack:d2],
         [=xmm11 -> stack:d1],
         [=xmm10 -> stack:d15],
         [=xmm9 -> stack:d14],
         [=xmm8 -> stack:d13],
         [=xmm7 -> stack:d12],
         [=xmm6 -> stack:d11],
         [=xmm5 -> stack:d10],
         [=xmm4 -> stack:d9],
         [=xmm3 -> stack:d8],
         [=xmm2 -> stack:d7],
         [=xmm1 -> stack:d6],
         [=xmm0 -> stack:d5],
         [=xmm12 -> stack:d3],
         [stack:d3 -> =xmm13] <|@
      21 goto () <|@
    end_LIR
Interestingly, this fails on both allocators.
Blocks: 677337
Attached patch wip v0 (obsolete) — Splinter Review
At the backedge of loops we store some information in the LDefinition::output field of phis. But this field is used to compute whether an instruction has canonical backing stack. So that's the first bug. But with this, the Greedy allocator still produces the wrong result - just now it's a normal number instead of NaN.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attached patch fixSplinter Review
One more bug. The MoveEmitter was incorrect when a cycle involved memory-to-memory doubles. It was detecting the size via the MoveOperand, when it actually has to use the MoveKind.

While fixing this, I made MoveEmitter use ScratchFloatReg instead. This got rid of a ton of spill logic.
Attachment #569545 - Attachment is obsolete: true
Attachment #570036 - Flags: review?
Attachment #570036 - Flags: review? → review?(sstangl)
Attached patch follow-up fix (obsolete) — Splinter Review
Heh. A third bug causing a failure in the same testcase, with GVN or LICM off. The problem is when instruction X has spill slot N. N gets freed before allocating registers, so it can be re-used as a spill slot during eviction. This messes up the spill/restore code. So I just moved stack slot killing later in the algorithm.
Attachment #570090 - Flags: review?(sstangl)
This fix botched on redefines. Here's the right fix.
Attachment #570090 - Attachment is obsolete: true
Attachment #570090 - Flags: review?(sstangl)
Attachment #570117 - Flags: review?(sstangl)
Attachment #570036 - Flags: review?(sstangl) → review+
Attachment #570117 - Flags: review?(sstangl) → review+
You need to log in before you can comment on or make changes to this bug.