Enable BaselineJIT on aarch64

RESOLVED FIXED in Firefox 53

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: denis.scott.v, Assigned: denis.scott.v)

Tracking

Trunk
mozilla53
Other
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.87 Safari/537.36
(Assignee)

Comment 1

2 years ago
Posted patch BaselineJIT-AARCH64.patch (obsolete) — Splinter Review
Attachment #8818149 - Flags: review?(bbouvier)
(Assignee)

Updated

2 years ago
OS: Unspecified → Linux
Hardware: Unspecified → Other
(Assignee)

Updated

2 years ago
Attachment #8818149 - Flags: review?(bbouvier) → review?(jdemooij)
(Assignee)

Comment 2

2 years ago
Comment on attachment 8818149 [details] [diff] [review]
BaselineJIT-AARCH64.patch

Move to Review Board.
Attachment #8818149 - Attachment is obsolete: true
Attachment #8818149 - Flags: review?(jdemooij)
(Assignee)

Comment 3

2 years ago
Comment on attachment 8818149 [details] [diff] [review]
BaselineJIT-AARCH64.patch

Upload patch to reviewboard failed :-(
Attachment #8818149 - Attachment is obsolete: false
Attachment #8818149 - Flags: review?(jdemooij)
Comment on attachment 8818149 [details] [diff] [review]
BaselineJIT-AARCH64.patch

Review of attachment 8818149 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch!

Looks good to me, but I'm requesting an additional review from Jakob, who was the last to work on the ARM64 code. Jakob, can you think of any reason not to do this?
Attachment #8818149 - Flags: review?(jolesen)
Attachment #8818149 - Flags: review?(jdemooij)
Attachment #8818149 - Flags: review+
(Assignee)

Comment 5

2 years ago
Thanks!
It seems jakob hasn't login bugzilla for a long time
Comment on attachment 8818149 [details] [diff] [review]
BaselineJIT-AARCH64.patch

Adding an additional review from Sean in case Jakob is on PTO or something.
Attachment #8818149 - Flags: review?(sstangl)
Comment on attachment 8818149 [details] [diff] [review]
BaselineJIT-AARCH64.patch

Review of attachment 8818149 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. The baseline JIT is tested with the simulator, so it should be OK. The Ion JIT does not work yet.
Attachment #8818149 - Flags: review?(jolesen) → review+
Assignee: nobody → denis.scott.v
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: checkin-needed
Attachment #8818149 - Flags: review?(sstangl)

Comment 8

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c216703bd7e2
Enable BaselineJIT on aarch64. r=jdemooij
Keywords: checkin-needed

Comment 9

2 years ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9dce91a5322a
Backed out changeset c216703bd7e2 for bustage
bustage -> https://treeherder.mozilla.org/logviewer.html#?job_id=40937538&repo=mozilla-inbound sorry had to back this out
Flags: needinfo?(denis.scott.v)
(Assignee)

Comment 11

2 years ago
I have no idea how to solve this issue. There is no problem on Linux-x64.
Flags: needinfo?(denis.scott.v)
I think the problem is that we #include "jit/arm64/vixl/Cpu-vixl.h" unconditionally and it doesn't compile on Windows. The code on line 103 in Instructions-vixl.h. I think it'd be okay for now to #include that header only if JS_CODEGEN_ARM64.
(Assignee)

Comment 13

2 years ago
Posted patch v2.patchSplinter Review
Attachment #8818149 - Attachment is obsolete: true
(Assignee)

Comment 14

2 years ago
Thanks!

Comment 16

2 years ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/da3671e1528c
Enable BaselineJIT on aarch64. r=jandem,jolesen

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/da3671e1528c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Updated

2 years ago
Depends on: 1359051

Updated

2 years ago
Depends on: 1359142
You need to log in before you can comment on or make changes to this bug.