Closed Bug 517939 Opened 15 years ago Closed 15 years ago

x64 regalloc_binary encounters LIR_u2f and LIR_float in GPRs as arguments, should we permit this?

Categories

(Tamarin Graveyard :: Baseline JIT (CodegenLIR), defect)

x86_64
All
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED DUPLICATE of bug 521691

People

(Reporter: rreitmai, Unassigned)

References

Details

The change http://hg.mozilla.org/tamarin-redux/rev/089d4ff557a7 introduced code to ensure that both arguments to regalloc_binary are in correct registers.

The change pulled directly from Tracemonkey resulted in asserting on line 1.24.  So LIR_u2f and LIR_float were added to the original code and this was checked-into tamarin.

The bug is open for discussion on how we wish this resolve the issue.
It sounds like asm_store64 should use FpRegs if it sees either of those
instructions.

The comment is confusing because I forgot to end the first line with a
period. Sorry about that.

-dvander

On 9/18/09 1:31 PM, Rick Reitmaier wrote:
> In regalloc_binary() on NativeX64 as change was made as
> a result of bug 516093.
>
> Unfortunately this breaks tamarins' acceptance suite for
> mac x64 support.
>
> Specifically :
>
>          } else if (!(allow&  rmask(ra))) {
>              // rA already has a register assigned, but it's not valid
>              // to make sure floating point operations stay in FPU registers
>              // as much as possible, make sure that only a few opcodes are
>              // reserving GPRs.
>              NanoAssert(a->opcode() == LIR_quad || a->opcode() == LIR_ldq);
>              allow&= ~rmask(rr);
>              ra = findRegFor(a, allow);
>          }
>
> asserts on a->opcode().
>
> If I extend the assert to include LIR_u2f and LIR_float the
> regressions pass, but this doesn't seem like a proper fix.
>
> The underlying issue is that an stqi leads to asm_store64()
> which assigns a GPR to the instruction.
>
> If this instruction is used a parameter in regalloc_binary() we assert.
>
> Thoughts?
>
> - Rick
Summary: x64 regalloc_binary encounters LIR_u2f and LIR_float as arguments, should we permit this? → x64 regalloc_binary encounters LIR_u2f and LIR_float in GPRs as arguments, should we permit this?
I hit this with the lirasm --random mode.  Changing asm_store64() as suggested by comment #1 is the short-term fix.

The proper fix is to distinguish float loads/stores from quad load/stores.

(Nb: bug 520208 is the TM version of this bug.)
(In reply to comment #2)
> 
> The proper fix is to distinguish float loads/stores from quad load/stores.

See bug 520714.
Bug 521691's patch fixes the symptom.  I'll mark this as a dup of that bug.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.