522755 - TM merge: asm_spilli changes from TM do not work on PPC tamarin

Status: VERIFIED FIXED

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

Description

[Rick Reitmaier]

On PPC (and it looks like only on this one configuration) adding the ins->isOp(LIR_iparam) code to test for quad causes PPC runs to assert.

Its not clear to me why this change was added to TM in the first
place as it seems incorrect to classify LIR_iparam as a quad.

Offending code exists in asm_spilli() see below: 

#ifdef TM_MERGE
        // fails on ppc see 
        bool quad = ins->opcode() == LIR_iparam || ins->isQuad();
#endif
        bool quad = /*ins->opcode() == LIR_iparam ||*/ ins->isQuad();

Comment 1

[Graydon Hoare :graydon]

You'll be happy to know its sordid history, then. A little friday-afternoon code archaeology:

changeset:   32199:cbb2bdb22a22
user:        Nicholas Nethercote <[e-mail]>
date:        Thu Sep 10 11:00:18 2009 +1000
summary:     Bug 514349 - nanojit: start to kill Reservations.  r=edwsmith,rreitmai.

-        bool quad = i->opcode() == LIR_iparam || i->isQuad();
+        bool quad = ins->opcode() == LIR_iparam || ins->isQuad();


changeset:   30295:a79667662e93
user:        Nicholas Nethercote <[e-mail]>
date:        Wed Jul 15 10:00:43 2009 +1000
summary:     Bug 504213: nanojit: rename LIR_alloc, LIR_addp, LIR_param to match tamarin-redux.  r=graydon.

-        bool quad = i->opcode() == LIR_param || i->isQuad();
+        bool quad = i->opcode() == LIR_iparam || i->isQuad();

changeset:   20565:292bfe66ddf9
user:        Edwin Smith <[e-mail]>
date:        Mon Oct 20 10:15:07 2008 -0700
summary:     fixes to enable ARM nanojit to build, at least

moved to Assembler.cpp from Nativei386.cpp

- bool quad = i->opcode() == LIR_param || i->isQuad();
+ bool quad = i->opcode() == LIR_param || i->isQuad();

changeset:   20437:53072c29a4fe
user:        Graydon Hoare
date:        Mon Oct 13 13:29:18 2008 -0700
summary:     Merge tamarin-redux (nanojit2) into tracemonkey (457786, r=edwsmith,gal,danderson).

IOW, it was born in tamarin-redux, we just blindly copied-and-mutated it forward ever since, without any particular commitment to the cause. 

Edwin eventually fixed it:

changeset:   1313:de598a38ae61
user:        Edwin Smith <[e-mail]>
date:        Fri Jan 16 21:03:17 2009 -0500
summary:     Add many more 64bit lir opcodes, and new arg types for CallInfo signatures

-               bool quad = i->opcode() == LIR_param || i->isQuad();
+               asm_spill(rr, d, pop, i->isQuad());

But we never absorbed that change. I'll take it now.

Comment 2

[Rick Reitmaier]

pushed http://hg.mozilla.org/tamarin-redux/rev/e30274ad430d