540368 - nanojit: split LIR_qlo, LIR_live and LIR_ret into two opcodes each to faciliate LIR type-checking

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Nicholas Nethercote [inactive]]

LIR_qlo currently has two distinct uses.  One is in combination with LIR_qhi
and LIR_qjoin, for splitting/joining 64-bit floats.  The other is for 
truncating 64-bit integers.  Because these have different operand types
LIR_qlo cannot be type-checked (see bug 463137).  We need to split these two
uses into two different opcodes.  Furthermore, the split-float64 version
should only occur on 32-bit (softfloat) platforms, but the truncate-int64
version should only occur on 64-bit platforms.

Also, LIR_live is currently used for both 32-bit integers and 64-bit 
integers, and so also cannot be type-checked.  It should be split into 
LIR_live and LIR_qlive.  And LIR_qlive shouldn't occur on 32-bit platforms.

Comment 1

[Nicholas Nethercote [inactive]]

LIR_ret is also a problem: it is used in TM as if it takes a 32-bit integer argument, but it is used in TR as if it takes a word-sized integer argument.  We should split it into LIR_ret and LIR_qret for clarity and consistency with all the other cases, and have LIR_pret as a synonym.  (We should probably also have LIR_plive as a synonym.)

Comment 2

[Andreas Gal :gal]

qparam/pparam/fparam is missing too.

Comment 3

[Nicholas Nethercote [inactive]]

(In reply to comment #2)
> qparam/pparam/fparam is missing too.

We have qparam, and pparam exists as a synonym, although it's called param.  We don't have fparam, but this bug is concerned with opcodes used on multiple types rather than missing opcodes (there are quite a few of those, but they don't seem that important if they're not being used).

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: NJ patch

All pretty straightforward.  The ExprFilter::ins1() changes are mostly because I renamed 'i' to the clearer 'oprnd1'.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: TR patch

Patch passes acceptance tests on i386 and X64 on my machine.  The
pret(i2p(x)) and pret(u2p(x)) cases look like they might be able to be
reduced to just ret(x) but I wasn't sure so I didn't try it.

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: TM patch

Comment 7

[Edwin Smith]

Comment on attachment 423696 [details] [diff] [review]
NJ patch

low-priority: on ppc64, if the input to LIR_q2i is in a GP register then it's essentially a mov.  Spilling and reloading will just insert memory traffic & stalls.  qlo had to do that (conservatively) for the case when the input is in a FP register.

Comment 8

[Edwin Smith]

(In reply to comment #5)
> Created an attachment (id=423697) [details]
> TR patch
> 
> Patch passes acceptance tests on i386 and X64 on my machine.  The
> pret(i2p(x)) and pret(u2p(x)) cases look like they might be able to be
> reduced to just ret(x) but I wasn't sure so I didn't try it.

On 32bit machines, they should reduce to just LIR_ret, but on 64bit machines they should cause zero-extension or sign-extension of the result.

The alternative would be to have LIR_uret and LIR_iret, which on 64bit machines would do the sign extension on ABI's that require the callee to do it.  (I haven't investigated)   ... or require LIR_ret on 64-bit cpu's to inspect the call signature and use that instead of the opcode.

Comment 9

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/projects/nanojit-central/rev/54cf6d39a21d

Comment 10

[Nicholas Nethercote [inactive]]

http:///hg.mozilla.org/tracemonkey/rev/ec77f24175bf
http:///hg.mozilla.org/tracemonkey/rev/3c3b005de959

Comment 11

[Steven Johnson]

http://hg.mozilla.org/tamarin-redux/rev/ac8300cb5a15

Comment 12

[Nicholas Nethercote [inactive]]

http:///hg.mozilla.org/mozilla-central/rev/ec77f24175bf
http:///hg.mozilla.org/mozilla-central/rev/3c3b005de959