Closed
Bug 540368
Opened 14 years ago
Closed 14 years ago
nanojit: split LIR_qlo, LIR_live and LIR_ret into two opcodes each to faciliate LIR type-checking
Categories
(Core Graveyard :: Nanojit, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin)
Attachments
(3 files)
35.06 KB,
patch
|
edwsmith
:
review+
|
Details | Diff | Splinter Review |
11.04 KB,
patch
|
edwsmith
:
review+
|
Details | Diff | Splinter Review |
3.20 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•14 years ago
|
||
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.)
Summary: nanojit: split LIR_qlo and LIR_live into two opcodes each → nanojit: split LIR_qlo, LIR_live and LIR_ret into two opcodes each
Comment 2•14 years ago
|
||
qparam/pparam/fparam is missing too.
Assignee | ||
Comment 3•14 years ago
|
||
(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).
Assignee | ||
Updated•14 years ago
|
Summary: nanojit: split LIR_qlo, LIR_live and LIR_ret into two opcodes each → nanojit: split LIR_qlo, LIR_live and LIR_ret into two opcodes each to faciliate LIR type-checking
Assignee | ||
Comment 4•14 years ago
|
||
All pretty straightforward. The ExprFilter::ins1() changes are mostly because I renamed 'i' to the clearer 'oprnd1'.
Attachment #423696 -
Flags: review?(edwsmith)
Assignee | ||
Comment 5•14 years ago
|
||
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.
Attachment #423697 -
Flags: review?(edwsmith)
Assignee | ||
Comment 6•14 years ago
|
||
Attachment #423698 -
Flags: review?(dvander)
Comment 7•14 years ago
|
||
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.
Attachment #423696 -
Flags: review?(edwsmith) → review+
Comment 8•14 years ago
|
||
(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.
Updated•14 years ago
|
Attachment #423697 -
Flags: review?(edwsmith) → review+
Updated•14 years ago
|
Attachment #423698 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 9•14 years ago
|
||
http://hg.mozilla.org/projects/nanojit-central/rev/54cf6d39a21d
Whiteboard: fixed-in-nanojit
Assignee | ||
Comment 10•14 years ago
|
||
http:///hg.mozilla.org/tracemonkey/rev/ec77f24175bf http:///hg.mozilla.org/tracemonkey/rev/3c3b005de959
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tracemonkey
Comment 11•14 years ago
|
||
http://hg.mozilla.org/tamarin-redux/rev/ac8300cb5a15
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey → fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin
Assignee | ||
Comment 12•14 years ago
|
||
http:///hg.mozilla.org/mozilla-central/rev/ec77f24175bf http:///hg.mozilla.org/mozilla-central/rev/3c3b005de959
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•