Closed
Bug 976648
Opened 11 years ago
Closed 11 years ago
Support powerpc64le-linux platform
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: uweigand, Assigned: uweigand)
References
Details
Attachments
(6 files, 6 obsolete files)
2.71 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
83.46 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
1.26 KB,
patch
|
jesup
:
review+
|
Details | Diff | Splinter Review |
1.61 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
17.35 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
64.48 KB,
patch
|
blassey
:
review+
|
Details | Diff | Splinter Review |
The new little-endian 64-bit PowerPC platform (powerpc64le-linux) is not currently supported by Mozilla. I'll attach a patch series to add support, consisting of the following parts:
1. Toplevel build/config support
2. NSPR build/config support
3. Javascript build/config support
4. js/src/ctypes - libffi backport
5. xptcall port
6. webrtc config support
7. mfbt config support
Assignee | ||
Comment 1•11 years ago
|
||
This patch adds toplevel build/config changes for powerpc64le-linux:
- Update config.guess / config.sub to current upstream versions
- Set CPU_ARCH in configure.in
Assignee | ||
Comment 2•11 years ago
|
||
This patch adds NSPR build/config changes for powerpc64le-linux:
- Update config.guess / config.sub to current upstream versions
- Correctly set endian macros in pr/include/md/_linux.cfg
Assignee | ||
Comment 3•11 years ago
|
||
This patch adds js/src build/config support for powerpc64le-linux:
- Set CPU_ARCH in configure.in
- Correctly set endian macros in jscpucfg.h
- Correctly set endian macros in assembler/wtf/Platform.h
Assignee | ||
Comment 4•11 years ago
|
||
This patch adds js/src/ctypes/libffi support for powerpc64le-linux:
- Fix libtool shared library handling on powerpc64le-linux
This backports just the minimal change necessary from upstream libtool into m4/libtool.m4 and the generated files aclocal.m4 and configure.
- Backport powerpc back-end support from upstream libffi
This backports all changes in src/powerpc, except those that affect solely AIX or Darwin, and except for the following patch (which would require backporting libffi common code changes, and which is not necessary for the built-in copy of libffi in Mozilla since it only implemented back-level compatibility support for a libffi.so shared library):
https://sourceware.org/ml/libffi-discuss/2013/msg00220.html
Overall, non-PowerPC platforms should be completely unaffected by this backport.
Once you upgrade libffi to current mainline, this patch becomes fully obsolete.
Assignee | ||
Comment 5•11 years ago
|
||
This patch adds xptcall support for powerpc64le-linux.
This primarily consists of support for the new ELFv2 ABI. The main changes introduced with this ABI that affect xptcall are the following:
- No more function descriptors
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01141.html
- Stack frame layout changes
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01149.html
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01146.html
In addition, the patch adds fixes to correctly locate "float" arguments on the stack in little-endian mode.
Finally, the patch adds moz.build support to enable the powerpc64le platform.
Note that I've had to rename the assembler files from .s to .S extensions to make sure they are run through the preprocessor.
Assignee | ||
Comment 6•11 years ago
|
||
This patch fixes the endian defines in media/webrtc/trunk/webrtc/typedefs.h.
Assignee | ||
Comment 7•11 years ago
|
||
This patch fixes the mfbt/Endian.h defines for little-endian PowerPC.
This is not strictly necessary on Linux, since there is a generic __GNUC__ clause that will get the endian macros correct when building with GCC, but this patch fixes the defines when building with a non-GCC compiler.
Assignee | ||
Updated•11 years ago
|
Attachment #8381536 -
Flags: review?(benjamin)
Assignee | ||
Updated•11 years ago
|
Attachment #8381538 -
Flags: review?(ted)
Assignee | ||
Updated•11 years ago
|
Attachment #8381542 -
Flags: review?(benjamin)
Assignee | ||
Updated•11 years ago
|
Attachment #8381546 -
Flags: review?(benjamin)
Assignee | ||
Updated•11 years ago
|
Attachment #8381549 -
Flags: review?(nfroyd)
Assignee | ||
Updated•11 years ago
|
Attachment #8381554 -
Flags: review?(rjesup)
Assignee | ||
Updated•11 years ago
|
Attachment #8381558 -
Flags: review?(jwalden+bmo)
Comment 8•11 years ago
|
||
Ulrich, should you be listed as the port maintainer?
Comment 9•11 years ago
|
||
Comment on attachment 8381536 [details] [diff] [review]
Part 1: Toplevel build/config
Are the config.guess changes just importing a new copy of config.guess from GNU config? r=me if so or re-request review if there's something special.
Attachment #8381536 -
Flags: review?(benjamin) → review+
Updated•11 years ago
|
Attachment #8381542 -
Flags: review?(benjamin) → review+
Updated•11 years ago
|
Attachment #8381546 -
Flags: review?(benjamin) → review+
Comment 10•11 years ago
|
||
Comment on attachment 8381549 [details] [diff] [review]
Part 5: xptcall port
Review of attachment 8381549 [details] [diff] [review]:
-----------------------------------------------------------------
This all looks fine.
::: xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc64_linux.S
@@ +17,5 @@
> .set f25,25; .set f26,26; .set f27,27; .set f28,28; .set f29,29
> .set f30,30; .set f31,31
>
> +#if _CALL_ELF == 2
> +#define STACK_TOC 28
For my own edification, why does this not match up with the RS6000_TOC_SAVE_SLOT/RS6000_SAVE_TOC macros in http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01146.html ? 28 seems like sort of an odd number in this context, insofar as it's not evenly divisible by the word size.
@@ +70,5 @@
> #
> # | ..128-byte stack frame.. | | 7 GP | 13 FP | 3 NV |
> # | |(params)........| regs | regs | regs |
> # (r1)...........(+112)....(+128)
> # (-23*8).(-16*8).(-3*8)..(r31)
This comment/diagram could probably stand some updating if you felt like it.
@@ +84,5 @@
> # uint32_t paramCount, nsXPTCVariant* s,
> # uint64_t* d))
>
> # r5, r6 are passed through intact (paramCount, params)
> # r7 (d) has to be r1+112 -- where parameters are passed on the stack.
You may want to update this 112 now?
Attachment #8381549 -
Flags: review?(nfroyd) → review+
Comment 11•11 years ago
|
||
Comment on attachment 8381558 [details] [diff] [review]
Part 7: mfbt endian defines
Review of attachment 8381558 [details] [diff] [review]:
-----------------------------------------------------------------
Stealing review!
Attachment #8381558 -
Flags: review?(jwalden+bmo) → review+
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•11 years ago
|
Attachment #8381554 -
Flags: review?(rjesup) → review+
Comment 12•11 years ago
|
||
Comment on attachment 8381554 [details] [diff] [review]
Part 6: WebRTC endian defines
Review of attachment 8381554 [details] [diff] [review]:
-----------------------------------------------------------------
Note: since this affects upstream code, an Issue should be opened with the upstream for this (webrtc.org: https://code.google.com/p/webrtc/issues/list) and the patch added to the webrtc codereview site. Once the issue is reported there, please update this bug with the Issue, and link this bug to webrtc_upstream_bugs (bug 864513)
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #9)
> Comment on attachment 8381536 [details] [diff] [review]
> Part 1: Toplevel build/config
>
> Are the config.guess changes just importing a new copy of config.guess from
> GNU config? r=me if so or re-request review if there's something special.
Yes, this just imports a new copy of config.guess / config.sub from GNU config.
Thanks for the review!
Assignee | ||
Comment 14•11 years ago
|
||
(In reply to Nathan Froyd (:froydnj) from comment #10)
> Review of attachment 8381549 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> This all looks fine.
>
> ::: xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc64_linux.S
> @@ +17,5 @@
> > .set f25,25; .set f26,26; .set f27,27; .set f28,28; .set f29,29
> > .set f30,30; .set f31,31
> >
> > +#if _CALL_ELF == 2
> > +#define STACK_TOC 28
>
> For my own edification, why does this not match up with the
> RS6000_TOC_SAVE_SLOT/RS6000_SAVE_TOC macros in
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01146.html ? 28 seems like
> sort of an odd number in this context, insofar as it's not evenly divisible
> by the word size.
Oops, that needs to be 24 of course ... not sure how that happened.
Thanks for the thorough review!
I'll submit an updated patch with this fix, and some comment enhancements, shortly.
Assignee | ||
Comment 15•11 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #12)
> Comment on attachment 8381554 [details] [diff] [review]
> Part 6: WebRTC endian defines
>
> Review of attachment 8381554 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Note: since this affects upstream code, an Issue should be opened with the
> upstream for this (webrtc.org: https://code.google.com/p/webrtc/issues/list)
> and the patch added to the webrtc codereview site. Once the issue is
> reported there, please update this bug with the Issue, and link this bug to
> webrtc_upstream_bugs (bug 864513)
The upstream WebRTC version of this file:
https://code.google.com/p/webrtc/source/browse/trunk/webrtc/typedefs.h
does not contain any support for PowerPC at all, this seems to be a private addition in the Mozilla sources to begin with.
Does this mean that there is no need to forward this patch to upstream after all, or were you asking me to submit the whole of PowerPC support (big- and little-endian) upstream?
Assignee | ||
Comment 16•11 years ago
|
||
Fixed ELFv2 STACK_TOC definition, and enhanced comments.
Attachment #8381549 -
Attachment is obsolete: true
Attachment #8382203 -
Flags: review?(nfroyd)
Comment 17•11 years ago
|
||
Comment on attachment 8382203 [details] [diff] [review]
Part 5: xptcall port
Review of attachment 8382203 [details] [diff] [review]:
-----------------------------------------------------------------
I think perhaps you forget to |hg qref| or similar? This looks identical to the previous patch.
Attachment #8382203 -
Flags: review?(nfroyd)
Assignee | ||
Comment 18•11 years ago
|
||
Attaching the correct file this time, sorry ...
Attachment #8382203 -
Attachment is obsolete: true
Attachment #8382226 -
Flags: review?(nfroyd)
Comment 19•11 years ago
|
||
Comment on attachment 8382226 [details] [diff] [review]
Part 5: xptcall port
Review of attachment 8382226 [details] [diff] [review]:
-----------------------------------------------------------------
::: xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc64_linux.S
@@ +23,5 @@
> +# save area of 8 doublewords (used for vararg routines), followed
> +# by space for parameters passed on the stack.
> +#
> +# We set STACK_TOC to the offset of the TOC pointer save area, and
> +# STACK_PARAMS to the offset of the first on-stack parameter.
This is great, thank you!
Attachment #8382226 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 20•11 years ago
|
||
I just noticed that there is completely separate "Product" category for NSPR ... Should I submit the NSPR part as a separate bugzilla in that category instead of just having it as one part here?
Comment 21•11 years ago
|
||
typedef.h changes were landed (with a few updates since) as bug 814693; we can let that be the placeholder for upstreaming all of the changes to this file.. thanks
Assignee | ||
Updated•11 years ago
|
Attachment #8381538 -
Flags: review?(ted)
Assignee | ||
Updated•11 years ago
|
Attachment #8381538 -
Attachment is obsolete: true
Assignee | ||
Comment 22•11 years ago
|
||
NSPR changes split off into separate bug now.
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 23•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5056b4811257
https://hg.mozilla.org/integration/mozilla-inbound/rev/183ca3e3aa20
https://hg.mozilla.org/integration/mozilla-inbound/rev/f03d534b1114
https://hg.mozilla.org/integration/mozilla-inbound/rev/5599e2798ee3
https://hg.mozilla.org/integration/mozilla-inbound/rev/773113609bdf
https://hg.mozilla.org/integration/mozilla-inbound/rev/73b38a329f97
Assignee: nobody → uweigand
Keywords: checkin-needed
Comment 24•11 years ago
|
||
Backed out for failures during configure:
https://tbpl.mozilla.org/php/getParsedLog.php?id=35422834&tree=Mozilla-Inbound
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/095f820877eb
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/9124399187b6
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fee414cc9b5b
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/4d5e948764b7
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/a60a6910f827
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/10992841577e
Comment 25•11 years ago
|
||
Build changes (particularly configure and autoconf) must have review from a build peer - please request review from them one you've fixed the cause of the backout. I imagine :glandium might be your best bet.
Comment 26•11 years ago
|
||
(Sorry just noticed the bsmedberg review; had glanced at a hgweb diff for the r= and must have looked at one of the later parts)
Assignee | ||
Comment 27•11 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #24)
> Backed out for failures during configure:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=35422834&tree=Mozilla-
> Inbound
Sorry for the breakage ...
So it seems the difference is that configure is called with "arm-linux-androideabi", which the old config.sub transforms to "arm-linux-android", while the new one transforms it to "arm-unknown-linux-androideabi". The latter is the standard behaviour of GNU config; the former is due to private patches in Mozilla's config.sub copy, which were introduced by Bug 617115 and later fixed again by Bug 672939.
I guess longer term it would be better to change the rest of the build infrastructure to cope with the now-standard version of the target triple ... For now, I can add back a private change to have config.sub again return "arm-linux-android".
Investigating the Mercury history further, there is yet another private change to config.sub, to support the -wince-winmo operating system (introduced by Bug 515748). This is still not supported in upstream GNU config. Again, I can add back the local change.
Finally, there was also a private change to config.guess to support Darwin on x86_64 (introduced by Bug 515002). This is now supported in upstream GNU config directly, so the private change shouldn't be necessary any more.
I'll prepare an updated patch with those two private config.sub changes back in.
Comment 28•11 years ago
|
||
> ... For now, I can add back a private change to have config.sub again
> return "arm-linux-android".
Yes, let's do that.
> Investigating the Mercury history further, there is yet another private
> change to config.sub, to support the -wince-winmo operating system
> (introduced by Bug 515748). This is still not supported in upstream GNU
> config. Again, I can add back the local change.
This is not important, you don't need to add it back in.
Assignee | ||
Comment 29•11 years ago
|
||
Respin of the config.sub patch to add back in two local changes. See previous comments for discussion ...
Assignee | ||
Comment 30•11 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #28)
> > ... For now, I can add back a private change to have config.sub again
> > return "arm-linux-android".
>
> Yes, let's do that.
>
> > Investigating the Mercury history further, there is yet another private
> > change to config.sub, to support the -wince-winmo operating system
> > (introduced by Bug 515748). This is still not supported in upstream GNU
> > config. Again, I can add back the local change.
>
> This is not important, you don't need to add it back in.
Oops, didn't see that in time ... I'll take them back out and respin another patch.
Assignee | ||
Updated•11 years ago
|
Attachment #8383792 -
Attachment is obsolete: true
Assignee | ||
Comment 31•11 years ago
|
||
Respin config.sub patch to add back in the local change for *-linux-android* handling (set vendor to "linux" and OS to "android").
Attachment #8381536 -
Attachment is obsolete: true
Assignee | ||
Comment 32•11 years ago
|
||
For reference, the only remaining changes between config.sub as introduced by the latest patch and the upstream GNU config are now:
--- /home/uweigand/config/config.sub 2014-02-28 10:47:47.210000004 -0600
+++ config.sub 2014-02-28 12:17:28.530009090 -0600
@@ -1347,6 +1347,9 @@
-gnu/linux*)
os=`echo $os | sed -e 's|gnu/linux|linux-gnu|'`
;;
+ *-android*)
+ os=-android
+ ;;
# First accept the basic system types.
# The portable systems comes first.
# Each alternative MUST END IN A *, to match a version number.
@@ -1777,6 +1780,9 @@
-vos*)
vendor=stratus
;;
+ -android*)
+ vendor=linux
+ ;;
esac
basic_machine=`echo $basic_machine | sed "s/unknown/$vendor/"`
;;
Assignee | ||
Updated•11 years ago
|
Attachment #8383803 -
Flags: review?(benjamin)
Comment 33•11 years ago
|
||
Comment on attachment 8383803 [details] [diff] [review]
Part 1: Toplevel build/config
Feel free to file a followup for using stock config.sub and fixing our configure bits.
Attachment #8383803 -
Flags: review?(benjamin) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 34•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/084e0311ff2a
https://hg.mozilla.org/integration/mozilla-inbound/rev/94ec75013788
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a5fff440cb5
https://hg.mozilla.org/integration/mozilla-inbound/rev/2789ef4e1223
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f0f28593331
https://hg.mozilla.org/integration/mozilla-inbound/rev/18f113ab1a35
Keywords: checkin-needed
And back out http://hg.mozilla.org/integration/mozilla-inbound/rev/49d61a8f1e5c for Android x86 build bustage: https://tbpl.mozilla.org/php/getParsedLog.php?id=35451055&tree=Mozilla-Inbound
Flags: needinfo?(uweigand)
Assignee | ||
Comment 36•11 years ago
|
||
So there's some progress, the arm-linux-android target is now recognized as it used to. However, the i386-linux-android target is mis-recognized as i386-pc-android :-(
This is an unfortunate interaction between the parts of config.sub that were added upstream to properly recognize Android targets, and my port of the Mozilla-private config.sub changes that recognized Android targets in a special (now non-standard) way.
To stay with our original plan of keeping the Mozilla non-standard target triples (arm-linux-android and i386-linux-android), I guess what we have to do is to:
- *remove* the config.sub parts that were added upstream to recognize Android, *and*
- add back the (now unmodified) Mozilla-private hack to recognize Android
I've prepared another iteration of the patch so we might try yet another attempt to land the patch series. I've verified that this version of config.sub regconizes "arm-linux-android" and "i386-linux-android" the way the rest of Mozilla expects it.
Flags: needinfo?(uweigand)
Assignee | ||
Comment 37•11 years ago
|
||
Here's the diff against upstream config.sub that implements the two changes described in the previous comment:
--- /home/uweigand/config/config.sub 2014-02-28 10:47:47.210000004 -0600
+++ config.sub 2014-03-04 13:25:15.700000007 -0600
@@ -115,7 +115,7 @@
# Here we must recognize all the valid KERNEL-OS combinations.
maybe_os=`echo $1 | sed 's/^\(.*\)-\([^-]*-[^-]*\)$/\2/'`
case $maybe_os in
- nto-qnx* | linux-gnu* | linux-android* | linux-dietlibc | linux-newlib* | \
+ nto-qnx* | linux-gnu* | linux-dietlibc | linux-newlib* | \
linux-musl* | linux-uclibc* | uclinux-uclibc* | uclinux-gnu* | kfreebsd*-gnu* | \
knetbsd*-gnu* | netbsd*-gnu* | \
kopensolaris*-gnu* | \
@@ -123,10 +123,6 @@
os=-$maybe_os
basic_machine=`echo $1 | sed 's/^\(.*\)-\([^-]*-[^-]*\)$/\1/'`
;;
- android-linux)
- os=-linux-android
- basic_machine=`echo $1 | sed 's/^\(.*\)-\([^-]*-[^-]*\)$/\1/'`-unknown
- ;;
*)
basic_machine=`echo $1 | sed 's/-[^-]*$//'`
if [ $basic_machine != $1 ]
@@ -1367,7 +1363,7 @@
| -udi* | -eabi* | -lites* | -ieee* | -go32* | -aux* \
| -chorusos* | -chorusrdb* | -cegcc* \
| -cygwin* | -msys* | -pe* | -psos* | -moss* | -proelf* | -rtems* \
- | -mingw32* | -mingw64* | -linux-gnu* | -linux-android* \
+ | -mingw32* | -mingw64* | -linux-gnu* \
| -linux-newlib* | -linux-musl* | -linux-uclibc* \
| -uxpv* | -beos* | -mpeix* | -udk* \
| -interix* | -uwin* | -mks* | -rhapsody* | -darwin* | -opened* \
@@ -1506,6 +1502,9 @@
-dicos*)
os=-dicos
;;
+ -android*)
+ os=-android
+ ;;
-nacl*)
;;
-none)
@@ -1777,6 +1776,9 @@
-vos*)
vendor=stratus
;;
+ *-android*|*-linuxandroid*)
+ vendor=linux-
+ ;;
esac
basic_machine=`echo $basic_machine | sed "s/unknown/$vendor/"`
;;
Assignee | ||
Comment 38•11 years ago
|
||
Next attempt to update config.guess/config.sub.
Attachment #8383803 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #8385543 -
Flags: review?(benjamin)
Comment 39•11 years ago
|
||
Comment on attachment 8385543 [details] [diff] [review]
Part 1: Toplevel build/config
Going to bump this to blassey as the android expert.
Attachment #8385543 -
Flags: review?(benjamin) → review?(blassey.bugs)
Updated•11 years ago
|
Attachment #8385543 -
Flags: review?(blassey.bugs) → review+
Comment 40•11 years ago
|
||
Just a note, there are a lot of whitespace only changes in the upstream patches. I don't see too much harm in that since they are upstream patches, but normally I'm not a huge fan of churn for churn's sake.
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 41•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5bd19cc57e6
https://hg.mozilla.org/integration/mozilla-inbound/rev/454f70687a8d
https://hg.mozilla.org/integration/mozilla-inbound/rev/e36b6f8655db
https://hg.mozilla.org/integration/mozilla-inbound/rev/cade02043db6
https://hg.mozilla.org/integration/mozilla-inbound/rev/97bef611d855
https://hg.mozilla.org/integration/mozilla-inbound/rev/e88cd5f09c3d
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e5bd19cc57e6
https://hg.mozilla.org/mozilla-central/rev/454f70687a8d
https://hg.mozilla.org/mozilla-central/rev/e36b6f8655db
https://hg.mozilla.org/mozilla-central/rev/cade02043db6
https://hg.mozilla.org/mozilla-central/rev/97bef611d855
https://hg.mozilla.org/mozilla-central/rev/e88cd5f09c3d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Comment 43•11 years ago
|
||
Un-bust comm-central: https://hg.mozilla.org/comm-central/rev/5a77ecc9cc09
You need to log in
before you can comment on or make changes to this bug.
Description
•