GEOS-ESM / ESMA-Baselibs

Base Libraries for the GEOS ESM
Apache License 2.0
1 stars 10 forks source link

Add support for flang #220

Open mathomp4 opened 1 month ago

mathomp4 commented 1 month ago

Due to successes by @tclune in looking at flang + GFE (with a shoutout to @rouson), we should work on getting flang supported in Baselibs.

Current progress:

-------+---------+---------+--------------
Config | Install |  Check  |   Package
-------+---------+---------+--------------
  ok   |   ok    |   --    | antlr2
  ok   |   ok    |   --    | gsl
  ok   |   ok    |   --    | jpeg
  ok   |   ok    |   --    | zlib
  ok   |   ok    |   --    | szlib
  ok   |   ok    |   --    | curl
  ok   |   ok    |   --    | hdf4
  ok   |   --    |   --    | hdf5
  --   |   --    |   --    | netcdf
  --   |   --    |   --    | netcdf-fortran
  --   |   --    |   --    | netcdf-cxx4
  ok   |   ok    |   --    | udunits2
  --   |   --    |   --    | nco
  --   |   --    |   --    | cdo
  --   |   --    |   --    | nccmp
  --   |   --    |   --    | libyaml
  --   |   --    |   --    | FMS
  --   |   --    |   --    | esmf
  ok   |   ok    |   --    | xgboost
  ok   |   ok    |   --    | GFE
  --   |   --    |   --    | hdfeos
  --   |   --    |   --    | hdfeos5
  --   |   --    |   --    | SDPToolkit
-------+---------+---------+--------------
mathomp4 commented 1 month ago

At the moment we have a couple issues. One, we need to update HDF5 as 1.10 has no idea what flang is. We've held on to 1.10 for now only because of the odd nf90_create issues we saw in GEOS with 1.14.

But, we know GEOS is happy with hdf5 1.14 from spack, so let's just update for testing.

However, I just tried to build HDF5 1.14.4.3 and I believe I hit the same bug as @mjklemm did in https://github.com/llvm/llvm-project/issues/89403. (Indeed, I was about to file a bug in llvm-compiler but, well, I was beaten by a good 3 months!)

This code was added in https://github.com/HDFGroup/hdf5/pull/4246 by @derobins via @brtnfld ad by the time stamp, I'm guessing this is in 1.14.4 range, so I'll back off to 1.14.3 before this code got in.

mjklemm commented 1 month ago

I have a patch for this ready hopefully soon. I was more work than anticipated, but the patch seems to now work for Linux. The Windows part is still in the works.

PR: https://github.com/llvm/llvm-project/pull/95388

PS: Don't try this yet, it's still in the works. I can ping here, once it's ready for a test.

mathomp4 commented 1 month ago

I have a patch for this ready hopefully soon. I was more work than anticipated, but the patch seems to now work for Linux. The Windows part is still in the works.

PR: llvm/llvm-project#95388

Thanks!

Note with 1.14.3:

flang-new: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
error: loc("/ford1/share/gmao_SIteam/Baselibs/ESMA-Baselibs-8.5.0/src/hdf5/fortran/test/tf_gen.F90":208:5): /ford1/share/gmao_SIteam/llvm-flang/tmp/llvm-src/llvm-project-main/flang/include/flang/Optimizer/Support/Utils.h:143: not yet implemented: intrinsic: REAL(KIND=2) in SPACING
LLVM ERROR: aborting
make[4]: *** [Makefile:1054: tf_gen.lo] Error 1

I search a bit and I find something on the hdf5 site by a developer of the name, huh, @mjklemm : https://github.com/HDFGroup/hdf5/issues/4419

So, yeah, I'm definitely late to this party! 😄

mathomp4 commented 1 month ago

Note: I'm also using autotools for HDF5 (for...historical reasons more than anything else) so I might be missing errors that CMake would find in different ways.

mjklemm commented 1 month ago

I have a patch for that, too. Problem is that Flang reports that it supports FP16 and BF16 types, which it does for regular arithmetics, but it does not provide all the intrinsic. SPACING being one of them.

I have a hacks patch that disables the tests in HDF5. Let me know if you'd like to get it and I will see how I can best provide it.

Logged at LLVM as https://github.com/llvm/llvm-project/issues/83894.

brtnfld commented 1 month ago

There are other issues with flang and HDF5, https://github.com/llvm/llvm-project/issues/83894 https://github.com/llvm/llvm-project/issues/56883

mathomp4 commented 4 weeks ago

Adding a comment here about a doc by @jeffhammond saying how he's built HDF5 with flang:

https://github.com/jeffhammond/HPCInfo/blob/master/docs/NetCFD_HDF5_Flang.md

I'll need to give that a try at some point.

jeffhammond commented 4 weeks ago

I'll note my effort is the same as AMD folks did two weeks ago.

brtnfld commented 3 weeks ago

Did all the Fortran HDF5 tests pass?

mathomp4 commented 3 weeks ago

Did all the Fortran HDF5 tests pass?

Haven't had a chance to test on my end yet. Other fires being put out at the moment. Whee!

mathomp4 commented 1 day ago

Did all the Fortran HDF5 tests pass?

@brtnfld Good news, bad news. First, I built flang-new as of yesterday:

> flang-new --version
flang-new version 20.0.0git (git@github.com:GMAO-SI-Team/build-llvm-flang.git a39b3317362451d53d98542a2760451767a772d8)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /ford1/share/gmao_SIteam/llvm-flang/2024-08-21/bin

With that and with HDF5 1.14.4.3 and using the "comment out REAL(2) and REAL(3) bits in hdf5/fortran/test/tf_gen.F90 (per @jeffhammond) I get past those. However, I now die at:

make[4]: Entering directory '/ford1/share/gmao_SIteam/Baselibs/ClangTest/ESMA-Baselibs-8.6.0-TEST/src/hdf5/fortran/testpar'
  PPFC     mpi_param.o
flang-new: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
error: loc("/ford1/share/gmao_SIteam/MPI/openmpi/5.0.4/llvm-flang-2024-08-21/lib/mpi_f08.mod":408:24): 'hlfir.declare' op of numeric, logical, or assumed type entity must not have length parameters
error: verification of lowering to FIR failed
make[4]: *** [Makefile:1337: mpi_param.o] Error 1

I see H5_MPI_LOGICAL_KIND in there, so I have the latest version of that file.

So I guess two things:

  1. Whose error is this? Flang's? I think "flang" when I see "hlfir".
  2. Is there a way around this? Or is this such an important test I don't want to get around it?
mathomp4 commented 1 day ago

I did check and I can do a use mpi_f08 version of Hello World, so it's not like mpi_f08 does not work. So that's nice.