ARM64: Move Lowering's visitConstant() to shared code

RESOLVED FIXED in Firefox 42

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: sstangl, Assigned: sstangl)

Tracking

(Blocks: 1 bug)

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Created attachment 8638177 [details] [diff] [review]
0001-Move-Lowering-s-visitConstant-to-shared-code.-r.patch

While implementing ARM64 Ion support, I am aiming to move all code that should have been shared across platforms into shared code. In particular, nbp enjoys this kind of patch, so likely all such reviews will be sent that way :)

10 files changed, 18 insertions(+), 105 deletions(-)
Attachment #8638177 - Flags: review?(nicolas.b.pierron)
(Assignee)

Updated

3 years ago
Assignee: nobody → sstangl
Comment on attachment 8638177 [details] [diff] [review]
0001-Move-Lowering-s-visitConstant-to-shared-code.-r.patch

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

Thanks for doing it :)

This would reduce the overhead of adding new architecture, and hopefully ensure that we keep improving all architectures at the same time.
Attachment #8638177 - Flags: review?(nicolas.b.pierron) → review+
(Assignee)

Comment 4

3 years ago
Ah, apparently I need to build locally with -Werror.
Flags: needinfo?(sstangl)
(Assignee)

Comment 5

3 years ago
Honestly, I have no idea what is causing this error. Every file that uses define() also includes "jit/shared/Lowering-shared-inl.h", which includes the definition of define().
(Assignee)

Comment 6

3 years ago
Created attachment 8639568 [details] [diff] [review]
0001-Fix-non-unified-build.patch

This follow-up fixes the missing "-inl.h" that caused the tree to burn. With this patch, a non-unified build (FILES_PER_UNIFIED_FILE=1) with --enable-warnings-as-errors builds and passes "make check-style".
Attachment #8639568 - Flags: review?(efaustbmo)

Comment 7

3 years ago
Comment on attachment 8639568 [details] [diff] [review]
0001-Fix-non-unified-build.patch

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

APPROVED.
Attachment #8639568 - Flags: review?(efaustbmo) → review+
https://hg.mozilla.org/mozilla-central/rev/05fd68b041a8
https://hg.mozilla.org/mozilla-central/rev/2ba20cbc9e74
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.