Skip to content
This repository has been archived by the owner on Sep 30, 2022. It is now read-only.

Patch configure to support PowerPC little endian #668

Merged
merged 1 commit into from
Oct 20, 2015
Merged

Patch configure to support PowerPC little endian #668

merged 1 commit into from
Oct 20, 2015

Conversation

nysal
Copy link
Member

@nysal nysal commented Oct 14, 2015

Open MPI v1.10 tarballs are generated with libtool 2.4.2
The earliest libtool release that supports PowerPC little endian
is 2.4.3. Patch configure to enable support for this architecture.
See open-mpi/ompi/issues/396 for details.

:bot:assign: @jsquyres
:bot:milestone:v1.10.1
:bot🏷️enhancement

@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-v1.8-pr/843/ for details.

@jsquyres
Copy link
Member

This should go on master, too, right?

We probably don't need it on master (i.e., it's only for the case where someone using an older Libtool -- and not on a Power machine with the adjustment shell script -- makes a tarball for a Power user), but if autogen.pl is otherwise the same between v1.10 and master, it might be good to keep them in sync.

...I just checked, a diff between master autogen.pl and v1.10 autogen.pl shows that they are identical. We should probably keep autogen.pl identical between master, v1.10, and v2.x.

👍

@nysal
Copy link
Member Author

nysal commented Oct 15, 2015

A minor update to the patch - Added a copyright statement, tweaked the verbose message and moved invocation of patch_autotools_output to the same location as master branch. I'll prepare a similar patch for master and v2.x branches. Thanks for the review @jsquyres

@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-v1.8-pr/851/ for details.

@Zhiming-Wang
Copy link
Member

@nysal
If the later version libtool which the bug is fixed will be used for master, there will be two cases for Power LE like this. Right, but not elegant.

Such as "configure"

... ...
ppcle-linux|powerpcle-linux)
LD="${LD-ld} -m elf64lppc"
;;
powerpcle-linux)
LD="${LD-ld} -m elf64lppc"
;;
powerpc-linux)
LD="${LD-ld} -m elf64ppc"
;;
... ...

Open MPI v1.10 tarballs are generated with libtool 2.4.2
The earliest libtool release that supports PowerPC little endian
is 2.4.3. Patch configure to enable support for this architecture.
@jsquyres
Copy link
Member

@Zhiming-Wang Good catch! It looks like @nysal updated to only patch if necessary.

@nysal
Copy link
Member Author

nysal commented Oct 15, 2015

RHEL is the odd one out, the patch fails as expected on the other LE platforms. The reason the patch misbehaves on RHEL is that their fix for LE in libtool.m4 is a slight departure from how its fixed in upstream libtool. Hence our regex unfortunately matches and incorrectly tries to apply the patch. So the fix I would like to try is to test for the string 'powerpc64le-linux)' and if there is a match do not try to apply the patch. This will work on RHEL and SLES. However Ubuntu has "powerpc64le-*)", so the test fails and we will try to apply the patch. But the regex will not match and hence patch is not applied. So we are good (theoretically) in all 3 cases.
@Zhiming-Wang is testing the modified fix

@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-v1.8-pr/854/ for details.

@nysal
Copy link
Member Author

nysal commented Oct 16, 2015

@Zhiming-Wang has confirmed that it works on RHEL, Ubuntu and SLES. So this seems good to go for 1.10.1.

@Zhiming-Wang
Copy link
Member

@nysal @jsquyres
Yes, it worked.

@jsquyres
Copy link
Member

@rhc54 This is good to for go for v1.10.1.

rhc54 pushed a commit that referenced this pull request Oct 20, 2015
Patch configure to support PowerPC little endian
@rhc54 rhc54 merged commit 729b913 into open-mpi:v1.10 Oct 20, 2015
@nysal nysal deleted the topic/powerpcle branch December 19, 2015 08:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants