Closed
Bug 606066
Opened 14 years ago
Closed 14 years ago
nanojit: some i386 backend clean-ups
Categories
(Core Graveyard :: Nanojit, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin)
Attachments
(1 file)
11.54 KB,
patch
|
rreitmai
:
review+
|
Details | Diff | Splinter Review |
This patch cleans up the i386 back-end a bit more.
- MODRMsib() and MODRMs() do exactly the same thing, even though they look
different (except the argument order is different). This patch removes
MODRMs(), but changes MODRMsib to look more like MODRMs() did.
- This patch also changes code like this:
_nIns -= 2;
_nIns[0] = uint8_t(c>>8);
_nIns[1] = uint8_t(c);
to this:
*(--_nIns) = uint8_t(c);
*(--_nIns) = uint8_t(c>>8);
which is the style used in most of the rest of the file.
- This patch renamed MUL as IMUL, which matches the i386 opcode naming. ("mul"
is quite different and much more limited than "imul".)
Attachment #484935 -
Flags: review?(rreitmai)
Comment 1•14 years ago
|
||
(In reply to comment #0)
> - This patch also changes code like this:
>
> _nIns -= 2;
> _nIns[0] = uint8_t(c>>8);
> _nIns[1] = uint8_t(c);
>
> to this:
>
> *(--_nIns) = uint8_t(c);
> *(--_nIns) = uint8_t(c>>8);
Agreed that consistency is good, but I suspect that the first style actually generates better / more compact code, at least on x86.
![]() |
Assignee | |
Comment 2•14 years ago
|
||
(In reply to comment #1)
> (In reply to comment #0)
> > - This patch also changes code like this:
> >
> > _nIns -= 2;
> > _nIns[0] = uint8_t(c>>8);
> > _nIns[1] = uint8_t(c);
> >
> > to this:
> >
> > *(--_nIns) = uint8_t(c);
> > *(--_nIns) = uint8_t(c>>8);
>
> Agreed that consistency is good, but I suspect that the first style actually
> generates better / more compact code, at least on x86.
I don't think native codegen is hot enough for it to matter. Also, I plan to file follow-up bugs where most of these cases are replaced with calls to new functions like SIB(), MODRM(), OPCODE(), so there'll only be a few places where this is relevant. We can argue about it again then :)
Comment 3•14 years ago
|
||
Comment on attachment 484935 [details] [diff] [review]
patch (against TM 55696:51ca3657de5b)
Agreed, performance of codegen rarely shows up on the radar and the value of consistency cannot be understated.
Attachment #484935 -
Flags: review?(rreitmai) → review+
![]() |
Assignee | |
Comment 4•14 years ago
|
||
http://hg.mozilla.org/projects/nanojit-central/rev/08f542f7fe1e
http://hg.mozilla.org/tracemonkey/rev/53ddd7c5944b
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey
Comment 5•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 6•14 years ago
|
||
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey → fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin
Updated•11 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•