Closed Bug 977685 Opened 6 years ago Closed 6 years ago

Support powerpc64le-linux platform in NSPR

Categories

(NSPR :: NSPR, defect, P1)

PowerPC
Linux
defect

Tracking

(Not tracked)

RESOLVED FIXED
4.10.5

People

(Reporter: uweigand, Assigned: uweigand)

References

Details

Attachments

(3 files, 3 obsolete files)

The new little-endian 64-bit PowerPC platform (powerpc64le-linux) is not currently supported by NSPR.  The changes required to add support are:

- Update config.guess / config.sub to current GNU config upstream versions
- Correctly set endian macros in pr/include/md/_linux.cfg

I'll attach a patch to implement those changes shortly.

Note that overall powerpc64le-linux support for Mozilla is tracked in Bug 976648.
Attached patch Add powerpc64le-linux support (obsolete) — Splinter Review
Blocks: 976648
Attachment #8383149 - Flags: review?(ted)
Comment on attachment 8383149 [details] [diff] [review]
Add powerpc64le-linux support

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

Ulrich: thank you very much for the patch.

Ted: please compare the changes to config.guess and config.sub in
this patch with the patch in bug 944920. Hopefully they are the
same.

::: pr/include/md/_linux.cfg
@@ +31,5 @@
>  
> +#ifdef __LITTLE_ENDIAN__
> +#define IS_LITTLE_ENDIAN 1
> +#undef  IS_BIG_ENDIAN
> +#else

Ted: bug 909194 has an alternative patch for this file,
which tests the __BYTE_ORDER__ macro instead. I like
this patch better. In the commit message, please remember
to recognize the contribution of the author of the
alternative patch.
Assignee: wtc → ted
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P1
Target Milestone: --- → 4.10.5
(In reply to Wan-Teh Chang from comment #2)

> Ted: please compare the changes to config.guess and config.sub in
> this patch with the patch in bug 944920. Hopefully they are the
> same.

They're not quite the same -- my patch updates to an even more
recent version of the GNU config files than Marcin's (which I
hadn't been aware of ...).

However, Marcin's patch is also recent enough to support powerpc64le,
so I don't mind which of the two is applied in the end.
In getting the patches for Bug 976648 landed, we ran into build failures on Android when updating the Mozilla master config.sub.  It turns out that the old config.sub was not a plain copy of the upstream GNU config file, but had private code to support Android in a non-standard manner.

To fix this build break, we decided to add back that non-standard handling of Android in config.sub.  This is included in the patch that finally landed (today) in mozilla-central.

To avoid similar problems when changing the config.sub copy in nsprpub, it would probably be best to use the same (modified) version that has now landed in Mozilla.  I'll update the patch accordingly ...
Attached patch Add powerpc64le-linux support (obsolete) — Splinter Review
Updated config.sub to correspond to current mozilla-central version, including non-standard handling of Android target triples.
Attachment #8383149 - Attachment is obsolete: true
Attachment #8383149 - Flags: review?(ted)
Sorry, accidentally attached wrong file ...  This one is against the NSPR repo and has the correct log message.
Attachment #8386774 - Attachment is obsolete: true
Ulrich,

Thank you for the patch. I am also updating NSPR's config.guess
and config.sub scripts in bug 695993. The timestamps in the files
show you and I updated to the same upstream revision of the scripts.
This file shows the differences between my (first) and your (second)
patches.

1. config.guess: the difference is an NSPR-only local change.

2. config.sub: the "winmo" stuff (for Windows Mobile) is still in
NSPR but has been removed in your (Mozilla's) copy. However, I can't
explain the android-related diffs. It seems that Mozilla's copy
deleted three android-related things from the upstream copy. The
last difference can be ignored: the -android* case was added to
different places (one before the -nacl* case, the other after).

I'm going to use my patch for config.guess and config.sub. As far
as powerpc64le is concerned, both patches are equally good.
Comment on attachment 8388279 [details] [diff] [review]
Diffs between my and Ulrich's patches for NSPR's config.guess and config.sub scripts

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

::: nspr-tip.3/nspr/build/autoconf/config.sub
@@ -124,5 @@
>      basic_machine=`echo $1 | sed 's/^\(.*\)-\([^-]*-[^-]*\)$/\1/'`
>      ;;
> -  android-linux)
> -    os=-linux-android
> -    basic_machine=`echo $1 | sed 's/^\(.*\)-\([^-]*-[^-]*\)$/\1/'`-unknown

I looked into these android-related differences more closely.
I found that these are all new code from the GNU config upstream,
that Mozilla deleted. The deletions are noted in bug 976648
comment 37. So I believe I'll need to remove these from NSPR's
config.sub script as well.
Now the differences between my and Ulrich's patches have been
reduced.
Attachment #8388279 - Attachment is obsolete: true
(In reply to Wan-Teh Chang from comment #9)
> Created attachment 8388295 [details] [diff] [review]
> Diffs between my and Ulrich's patches for NSPR's config.guess and config.sub
> scripts, v2
> 
> Now the differences between my and Ulrich's patches have been
> reduced.

Except for the no-op android change, the only remaining changes I see in config.sub are related to -winmo.

This was a Mozilla-private change (apparently no longer needed) that I've removed as requested by bsmedberg in comment 28 of Bug 976648.
config.guess and config.sub were updated by the patch in bug 695993:
https://hg.mozilla.org/projects/nspr/rev/c8b3d4100698
Assignee: ted → uweigand
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.