Fix support for passing gcc-toolchain for clang compiler to LBANN SW stack #28769
Conversation
a gcc-toolchain cxxflags argument and pass it to a CMAKE_CUDA_FLAG argument when set. This helps deal with compiling with clang on systems with old base gcc installations. Added a dependency on py-scipy when enabling tests on LBANN. Updated the C++ standard for Hydrogen to C++17.
are used by applications in the LBANN repo, but are not strictly required for building and using LBANN. Updates several Python package dependencies to use the +app variant. Added the py-tqdm package to support the new RoBERTa application. Added a run time dependency for both py-pytest and py-scipy so that they are activated in any environment.
IBM ESSL BLAS library. This requires explicit identification of additional LAPACK libraries, since ESSL does not implement LAPACK, but is found by CMake. Fixed a bug in the LBANN dependency on OpenCV for Power architectures. The +powerpc variant is only required for GCC toolchains and causes Clang to break. Switched to only enabling when using %gcc on power.
bvanessen
force-pushed
the
bugfix_cuda_gcc_toolchain
branch
from
050d37b
to
aaa6509
Compare
| if self.spec['essl'] in spec: | ||
| args.extend([ | ||
| '-DLAPACK_LIBRARIES=%s;-llapack;-lblas' % self.spec['essl'].libs, | ||
| '-DBLAS_LIBRARIES=%s;-lblas' % self.spec['essl'].libs]) |
There was a problem hiding this comment.
Note that str(self.spec['essl'].libs) would only likely work if it has a single entry: by default it uses ' '.join(libs) so I think the resulting variable setting would be invalid if there were 2 or more libraries).
There was a problem hiding this comment.
That is a good point, but at the moment, ESSL really only exports one library. I also think that this is really more of a bug in the ESSL package or in CMake's find BLAS/LAPACK functions.
There was a problem hiding this comment.
The API of LibraryList will insert spaces. I think if you do % ';'.join(self.spec[essl].libs.names) it would future proof it to any libraries being added.
There was a problem hiding this comment.
Or wait that by itself won't work since .names doesn't convert to -lfoo just foo. So
';'.join('-l{0}'.format(lib) for lib in self.spec[essl].libs.names)
There was a problem hiding this comment.
Thanks @scheibelp I have applied these changes.
| if self.spec.satisfies('%clang'): | ||
| for flag in self.spec.compiler_flags['cxxflags']: | ||
| if 'gcc-toolchain' in flag: | ||
| args.append('-DCMAKE_CUDA_FLAGS=-Xcompiler={0}'.format(flag)) |
There was a problem hiding this comment.
(no change needed) it seems like this sort of thing might be worth extracting into some common class, since I figure this would apply to packages in general (not just these ones specifically). Presumably there are other flags we might want to pass along with Xcompiler as well.
There was a problem hiding this comment.
I agree that this may be a common issue and should really be in the CUDA package, but I didn't know how broadly that would be accepted.
…if there are multiple libraries for the ESSL package.
|
The style checker is complaining about line lengths in a few spots, but other than that this looks great to me. |
|
@scheibelp I fixed the flake8. Thanks for the help. |
|
Thanks! |
* Added support to LBANN, Hydrogen, DiHydrogen, and Aluminum to capture a gcc-toolchain cxxflags argument and pass it to a CMAKE_CUDA_FLAG argument when set. This helps deal with compiling with clang on systems with old base gcc installations. * Added a dependency on py-scipy when enabling tests on LBANN. * Updated the C++ standard for Hydrogen to C++17. * Added a new variant +apps to enable (or disable) python packages that are used by applications in the LBANN repo, but are not strictly required for building and using LBANN. * Added a run time dependency for both py-pytest and py-scipy so that they are activated in any environment. * Added support for building LBANN, Hydrogen, and DiHydrogen with the IBM ESSL BLAS library. This requires explicit identification of additional LAPACK libraries, since ESSL does not implement LAPACK, but is found by CMake. * Fixed a bug in the LBANN dependency on OpenCV for Power architectures. The +powerpc variant is only required for GCC toolchains and causes Clang to break. Switched to only enabling when using %gcc on power.
Added support to LBANN, Hydrogen, DiHydrogen, and Aluminum to capture
a gcc-toolchain cxxflags argument and pass it to a CMAKE_CUDA_FLAG
argument when set. This helps deal with compiling with clang on
systems with old base gcc installations.
Added a dependency on py-scipy when enabling tests on LBANN.
Updated the C++ standard for Hydrogen to C++17.