Last Comment Bug 655209 - Sparc build broken / Makefile not enabling arch for methodjit files
: Sparc build broken / Makefile not enabling arch for methodjit files
Status: RESOLVED FIXED
wanted-standalone-js
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: Sun Solaris
: -- blocker (vote)
: ---
Assigned To: Andrew Paprocki
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-05 23:53 PDT by Andrew Paprocki
Modified: 2011-10-24 19:29 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add sparc arch to list of methodjit archs (702 bytes, text/x-patch)
2011-05-05 23:53 PDT, Andrew Paprocki
no flags Details
Add sparc arch to list of methodjit archs (702 bytes, patch)
2011-05-05 23:54 PDT, Andrew Paprocki
ted: review-
Details | Diff | Splinter Review

Description Andrew Paprocki 2011-05-05 23:53:01 PDT
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.
Comment 1 Andrew Paprocki 2011-05-05 23:54:59 PDT
Created attachment 530555 [details] [diff] [review]
Add sparc arch to list of methodjit archs
Comment 2 Ted Mielczarek [:ted.mielczarek] 2011-05-12 09:05:55 PDT
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 Ted Mielczarek [:ted.mielczarek] 2011-05-12 09:16:36 PDT
I conferred with pbiggar and he thinks you can just replace that ugly filter line with
ifdef ENABLE_METHODJIT
Comment 4 Andrew Paprocki 2011-10-24 19:29:31 PDT
This was changed to ENABLE_METHOJIT in some other rev.

Note You need to log in before you can comment on or make changes to this bug.