implement NJ_EXPANDED_LOADSTORE_SUPPORTED for x64 backend

RESOLVED FIXED in Future

Status

Core Graveyard
Nanojit
RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: Steven Johnson, Assigned: Steven Johnson)

Tracking

unspecified
Future
x86_64
All

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-nanojit fixed-in-tamarin, fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

18.55 KB, patch
Edwin Smith
: review+
njn
: review+
Details | Diff | Splinter Review
Comment hidden (empty)
(Assignee)

Comment 1

8 years ago
Created attachment 416667 [details] [diff] [review]
Patch

Initial implementation.
Assignee: nobody → stejohns
Attachment #416667 - Flags: review?(edwsmith)
(Assignee)

Updated

8 years ago
Attachment #416667 - Flags: review?(nnethercote)
Comment on attachment 416667 [details] [diff] [review]
Patch

>@@ -438,6 +443,8 @@
>+    void Assembler::CVTSS2SD(R l, R r)  { emitprr(X64_cvtss2sd,l,r); asm_output("cvtss2sd %s, %s",RQ(l),RL(r)); }
>+    void Assembler::CVTSD2SS(R l, R r)  { emitprr(X64_cvtsd2ss,l,r); asm_output("cvtsd2ss %s, %s",RQ(l),RL(r)); }

I think the RQ/RL calls should be reversed in the second one.


>+    void Assembler::MOVBMR(R r1, I d, R r2)     { emitrm8(X64_movbmr,r1,d,r2); asm_output("movb %d(%s), %s",d,RQ(r1),RL(r2)); }
>+    void Assembler::MOVSMR(R r1, I d, R r2)     { emitrm_wide(X64_movsmr,r1,d,r2); asm_output("movs %d(%s), %s",d,RQ(r1),RL(r2)); }

For the first one, RL(r2) should be RB(r2), I think.  And we need to add RS() for the second one.

It's probably worth re-checking all the other RQ/RL calls for similar problems.

With those fixed, r=me.
Attachment #416667 - Flags: review?(nnethercote) → review+
(Assignee)

Comment 3

8 years ago
Comment on attachment 416667 [details] [diff] [review]
Patch

buggy, new patch coming soon
Attachment #416667 - Attachment is obsolete: true
Attachment #416667 - Flags: review?(edwsmith)
(Assignee)

Comment 4

8 years ago
Created attachment 416813 [details] [diff] [review]
Patch #2

Improved (functional) patch.
Attachment #416813 - Flags: review?(edwsmith)
(Assignee)

Updated

8 years ago
Attachment #416813 - Flags: review?(nnethercote)
(Assignee)

Updated

8 years ago
Depends on: 533854

Updated

8 years ago
Attachment #416813 - Flags: review?(edwsmith) → review+
(Assignee)

Comment 5

8 years ago
pushed to nj-c as changeset:   1106:cd0b46495c95
(Assignee)

Comment 6

8 years ago
pushed to tr as changeset:   3309:61d155d042c7
http://hg.mozilla.org/tracemonkey/rev/c1206e65e43f (on December 11, 2009)
http://hg.mozilla.org/tracemonkey/rev/55d3f8fc69fc
Whiteboard: fixed-in-nanojit fixed-in-tamarin, fixed-in-tracemonkey
(Assignee)

Comment 8

8 years ago
Pretty sure that this has been pushed to all relevant repos, shall we close it?
(Assignee)

Comment 9

8 years ago
Nick, since this is (pretty much) closed out, shall I remove your outstanding r?
Comment on attachment 416813 [details] [diff] [review]
Patch #2

Sorry, missed that.  Fixed now.
Attachment #416813 - Flags: review?(nnethercote) → review+

Updated

8 years ago
Component: JIT Compiler (NanoJIT) → Nanojit
Product: Tamarin → Core
QA Contact: nanojit → nanojit
Target Milestone: --- → Future
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Component: Nanojit → Nanojit
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.