Closed
Bug 676585
Opened 13 years ago
Closed 13 years ago
fix JS compilation for Darwin/ARM
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: dougt, Assigned: ted)
Details
(Whiteboard: [iOS][fixed-in-bs][fixed-in-nanojit])
Attachments
(1 file, 2 obsolete files)
5.23 KB,
patch
|
mjrosenb
:
review+
|
Details | Diff | Splinter Review |
ted: -#include <prtypes.h> +//#include <prtypes.h> I am not sure why this change.
Reporter | ||
Updated•13 years ago
|
Whiteboard: [iOS]
Reporter | ||
Comment 1•13 years ago
|
||
Attachment #550750 -
Flags: review?(cdleary)
Comment 2•13 years ago
|
||
Comment on attachment 550750 [details] [diff] [review] build on ios complication fixes Passing over to Marty, who has the ARM builds going these days.
Attachment #550750 -
Flags: review?(cdleary) → review?(mrosenberg)
Comment 3•13 years ago
|
||
Comment on attachment 550750 [details] [diff] [review] build on ios complication fixes Review of attachment 550750 [details] [diff] [review]: ----------------------------------------------------------------- I don't see anything wrong with the patch per se. It doesn't look like it breaks any existing code, and I can only assume that it builds firefox correctly for ios, so lgtm
Attachment #550750 -
Flags: review?(mrosenberg) → review+
Assignee | ||
Updated•13 years ago
|
Attachment #550749 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Assignee: doug.turner → ted.mielczarek
Assignee | ||
Updated•13 years ago
|
OS: FreeBSD → iOS 4
Hardware: All → ARM
Assignee | ||
Comment 4•13 years ago
|
||
I hit a snag actually trying to build with this patch on an updated tree. I'll put a new patch up soon.
Assignee | ||
Comment 5•13 years ago
|
||
Okay, I had to merge in some bits from some other patches I had laying around. I'll comment specifically on some of the bits here, since they're non-obvious.
Attachment #550750 -
Attachment is obsolete: true
Attachment #554141 -
Flags: review?
Assignee | ||
Comment 6•13 years ago
|
||
Comment on attachment 554141 [details] [diff] [review] js arm-darwin fixes Review of attachment 554141 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/assembler/jit/ExecutableAllocator.h @@ +374,4 @@ > _flush_cache(reinterpret_cast<char*>(code), size, BCACHE); > #endif > } > +#elif WTF_CPU_ARM && WTF_OS_IOS We never set WTF_CPU_ARM_THUMB2 (different from upstream webkit), so this code wouldn't get built previously. ::: js/src/assembler/wtf/Platform.h @@ +329,4 @@ > /* WTF_CPU_ARMV5_OR_LOWER - ARM instruction set v5 or earlier */ > /* On ARMv5 and below the natural alignment is required. > And there are some other differences for v5 or earlier. */ > +#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && !(WTF_ARM_ARCH_VERSION >= 6) This is just wrong, I don't know if it's necessary, but it doesn't match upstream certainly. @@ +1115,5 @@ > /* Setting this flag prevents the assembler from using RWX memory; this may improve > security but currectly comes at a significant performance cost. */ > #if WTF_PLATFORM_IOS > +//XXX: this doesn't currently compile in the spidermonkey build > +#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 0 The build breaks with this defined, maybe an artifact of our build system, but it's easier to just turn this off for now. ::: js/src/configure.in @@ -2006,4 @@ > STRIP="$STRIP -x -S" > _PLATFORM_DEFAULT_TOOLKIT='cairo-cocoa' > TARGET_NSPR_MDCPUCFG='\"md/_darwin.cfg\"' > - LDFLAGS="$LDFLAGS -framework Cocoa -lobjc" Turns out this is totally unnecessary. Probably just copy/pasted from the top-level configure.
Assignee | ||
Updated•13 years ago
|
Attachment #554141 -
Flags: review? → review?(mrosenberg)
Comment 7•13 years ago
|
||
Comment on attachment 554141 [details] [diff] [review] js arm-darwin fixes >-#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && WTF_ARM_ARCH_VERSION >= 6 >+#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && !(WTF_ARM_ARCH_VERSION >= 6) This seems rather bad. >-#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 1 >+//XXX: this doesn't currently compile in the spidermonkey build >+#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 0 I assume this is done through a system call we haven't bothered to research/use
Attachment #554141 -
Flags: review?(mrosenberg) → review+
Assignee | ||
Comment 8•13 years ago
|
||
(In reply to Marty Rosenberg [:Marty] from comment #7) > Comment on attachment 554141 [details] [diff] [review] > js arm-darwin fixes > > > >-#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && WTF_ARM_ARCH_VERSION >= 6 > >+#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && !(WTF_ARM_ARCH_VERSION >= 6) > This seems rather bad. I don't know how that got flipped from upstream. It probably hasn't been hurting anything because we force the use of the standard ARM backend and not the thumb backend. > >-#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 1 > >+//XXX: this doesn't currently compile in the spidermonkey build > >+#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 0 > I assume this is done through a system call we haven't bothered to > research/use No, I think it's just a build configuration that we haven't tested, so it doesn't quite build properly. We can probably make it work, it just seemed simpler to disable it for the moment.
Assignee | ||
Comment 9•13 years ago
|
||
http://hg.mozilla.org/projects/build-system/rev/06defc1b4250
Whiteboard: [iOS] → [iOS] fixed-in-bs
Comment 10•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/06defc1b4250
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Comment 11•13 years ago
|
||
Ok, bad news. Nanojit changes have to be landed on nanojit-central and then merged to mozilla-central. See https://developer.mozilla.org/en/NanojitMerge for details. The important thing is that if you don't do that, any Nanojit changes that land on mozilla-central get clobbered when the next Nanojit merge happens. I just did one, and the changes from this bug got clobbered. I can reland the Nanojit part of this patch in nanojit-central and do a merge. Is it urgent? (Nanojit changes are supposed to get a review from an Adobe person too, but these ones are innocuous enough that we can probably let it slide, though I've CC'd Edwin Smith just in case.)
Comment 12•13 years ago
|
||
Informal R+ from me.
Comment 13•13 years ago
|
||
(In reply to Edwin Smith from comment #12) > Informal R+ from me. Ok, thanks, I'll land and merge this later today.
Comment 14•13 years ago
|
||
http://hg.mozilla.org/projects/nanojit-central/rev/d66d2e24ef16
Whiteboard: [iOS] fixed-in-bs → [iOS][fixed-in-bs][fixed-in-nanojit]
Comment 15•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/6974e199548d
Whiteboard: [iOS][fixed-in-bs][fixed-in-nanojit] → [iOS][fixed-in-bs][fixed-in-nanojit][inbound]
Assignee | ||
Comment 16•13 years ago
|
||
Thanks, sorry about that!
Comment 17•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/6974e199548d
Whiteboard: [iOS][fixed-in-bs][fixed-in-nanojit][inbound] → [iOS][fixed-in-bs][fixed-in-nanojit]
You need to log in
before you can comment on or make changes to this bug.
Description
•