ompi icon indicating copy to clipboard operation
ompi copied to clipboard

configury: patch configure to support nvfortran

Open ggouaillardet opened this issue 1 year ago • 7 comments

nvfortran needs to be passed -fPIC when building shared libraries, so patch the generated configure script in order to properly handle nvfortran:

  • add nvfortran to the list of known fortran compilers
  • pass -fPIC to the compiler

Refs. #8919

ggouaillardet avatar Jul 30 '24 02:07 ggouaillardet

@cparrott73 can you please give it a try?

you should now be able to use nvfortran without having to manually pass FCFLAGS=-fPIC to configure. Once you apply the patch, you will have to run ./autogen.pl (or ./autogen.pl --force if you apply the patch on an official tarball.

ggouaillardet avatar Jul 30 '24 02:07 ggouaillardet

@ggouaillardet - I tried the patch with nvfortran here, but ./configure is still not passing -fPIC by default for me here.

Here is what I did:

  • Clone ompi from git via git clone --recursive https://github.com/open-mpi/ompi.git
  • Downloaded the above patch to a raw .diff file
  • cd ompi then patch -p1 < fpic_fix.diff
  • ./autogen.pl --force
  • mkdir build && cd build
  • ../configure CC=nvc FC=nvfortran CXX=nvc++

Did I not do something right here?

The system in question is a Supermicro H11DSU-iN AMD EPYC 7452 system running Rocky Linux 8.10. The following relevant packages are installed:

cparrott@rome5 /scratch/cparrott/ompi_fpic_test/ompi/build $ rpm -qa | grep autoconf
autoconf-2.69-29.el8.noarch
cparrott@rome5 /scratch/cparrott/ompi_fpic_test/ompi/build $ rpm -qa | grep automake
automake-1.16.1-8.el8.noarch

Let me know if I need to tweak something here.

cparrott73 avatar Jul 31 '24 19:07 cparrott73

That sounds right, I also use auto tools provided by RHEL8 so the patch should be applied correctly. Can you please compress and upload configure, config.log and libtool so I can have a look?

ggouaillardet avatar Aug 01 '24 02:08 ggouaillardet

@ggouaillardet - I tried this again while ago. I think my previous patch file from above may have gotten corrupted somehow. I pulled a new copy of the source tree from git, and then a new copy of the patch from above, and applied it, then ran ./configure and make. Everything worked as expected this time.

Interestingly, I also tried this on our older CentOS 7 build system (due to be retired soon). The change did not work as expected there. I'm wondering if this could be due to bugs/issues with autoconf and automake on that particular OS?

I also made a version of this patch for openmpi-4.1.5 by stripping out the first hunk, where the patch fails because of substantial changes in the copyright text since that version. This also worked okay for me on our Rocky 8 system.

All in all, I think we're good to go, and this experience has been educational to me - particularly with some of the shortcomings of CentOS 7.

cparrott73 avatar Aug 01 '24 05:08 cparrott73

I confirm it works on Rocky 9.4 as well both x86 and aarch64.

bosilca avatar Aug 01 '24 05:08 bosilca

@cparrott73 since we try to patch the generated configure and silently fail if the patch does not apply, the libtool.m4 used to generate configure must have the same bits than the one used to generate the patch. I generated it from a RHEL8 like box. If you want to use CentOS7, I suggest you manually apply the commit, autogen.pl and then make dist on a RHEL8 box, and then build from the tarball on CentOS7.

ggouaillardet avatar Aug 01 '24 10:08 ggouaillardet

@ggouaillardet @jsquyres can we get this merged?

janjust avatar Aug 15 '24 16:08 janjust

I tested again on x86 and aarch64 and it works with NVIDIA toolchains 24.7 and 24.9 (as long as --enable-alt-short-float=no is passed to configure on aarch64).

bosilca avatar Oct 22 '24 18:10 bosilca

@bosilca should we cherry-pick this to v5.0.x as well?

janjust avatar Oct 23 '24 03:10 janjust

@bosilca should we cherry-pick this to v5.0.x as well?

Yes, probably so.

Do you want it in v4.1.x, too?

jsquyres avatar Oct 23 '24 13:10 jsquyres

@jsquyres we should have it there as well

janjust avatar Oct 23 '24 14:10 janjust