Sparc build broken / Makefile not enabling arch for methodjit files

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
blocker
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Andrew Paprocki, Assigned: Andrew Paprocki)

Tracking

Trunk
Sun
Solaris
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: wanted-standalone-js)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 530553 [details]
Add sparc arch to list of methodjit archs

Now that the Sparc methodjit support has landed on m-c, the build is broken because ENABLE_METHODJIT and ENABLE_ASSEMBLER are defined, but the Makefile does not include 'sparc' in the list of supported architectures. The assembler files are not included in the .a file and the link fails.

This patch simply adds 'sparc' to the list of archs in the Makefile so that the build works. Confirmed this allows my js shell to link.
(Assignee)

Comment 1

6 years ago
Created attachment 530555 [details] [diff] [review]
Add sparc arch to list of methodjit archs
Attachment #530553 - Attachment is obsolete: true
Attachment #530555 - Flags: review?(ted.mielczarek)
Assignee: general → andrew
Comment on attachment 530555 [details] [diff] [review]
Add sparc arch to list of methodjit archs

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

::: js/src/Makefile.in
@@ +415,4 @@
>  # END enclude sources for V8 dtoa
>  #############################################
>  
> +ifeq (,$(filter arm% %86 x86_64 sparc,$(TARGET_CPU)))

This is getting unbearable. Should this just be ENABLE_METHODJIT or something like that?

@@ +472,5 @@
>  ifeq (arm, $(TARGET_CPU))
>  #CPPSRCS		+= only_on_arm.cpp
>  endif
> +ifeq (sparc, $(TARGET_CPU))
> +#CPPSRCS		+= only_on_sparc.cpp

What's the point of this commented-out bit? Are you just cargo-culting from above? I don't think we should bother having useless blocks here.
Attachment #530555 - Flags: review?(ted.mielczarek) → review-
I conferred with pbiggar and he thinks you can just replace that ugly filter line with
ifdef ENABLE_METHODJIT

Updated

6 years ago
Whiteboard: wanted-standalone-js
(Assignee)

Comment 4

6 years ago
This was changed to ENABLE_METHOJIT in some other rev.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.