655209 - Sparc build broken / Makefile not enabling arch for methodjit files

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andrew Paprocki]

Attachment: 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.

Comment 1

[Andrew Paprocki]

Attachment: Add sparc arch to list of methodjit archs

Comment 2

[(not currently active) Ted Mielczarek]

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.

Comment 3

[(not currently active) Ted Mielczarek]

I conferred with pbiggar and he thinks you can just replace that ugly filter line with
ifdef ENABLE_METHODJIT

Comment 4

[Andrew Paprocki]

This was changed to ENABLE_METHOJIT in some other rev.