Add support for clang-cuda #543

upsj · 2020-05-22T12:43:18Z

This PR takes the first steps to full clang-cuda support in Ginkgo. It works with the current CMake master version and (slightly patched) clang-10 (llvm/llvm-project@8e20516)

tcojean

LGTM. I guess you will have to update the CudaArchitecture commit once the PR is merged.

thoasm

LGTM!

upsj · 2020-05-22T23:25:48Z

I'll mark this as WIP until the issues with clang-cuda and HIP are solved. It looks like using HIP with clang also for host code won't work because nvcc doesn't like clang > 9.

upsj · 2020-05-23T08:51:06Z

Finally two issues: FindThreads doesn't deal with clang-cuda correctly (soon to be fixed) and the CUDA compiler was not propagated on installation, so it needed to be set in GinkgoConfig.

upsj · 2020-05-23T09:56:49Z

Since clang-cuda doesn't use a host compiler, the incompatibilities between Intel and clang/gcc symbol generation for constexpr globals (CI job) makes the clang-cuda/intel combination infeasible.

codecov · 2020-05-23T13:09:44Z

Codecov Report

Merging #543 into develop will decrease coverage by 0.00%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #543      +/-   ##
===========================================
- Coverage    89.15%   89.14%   -0.01%     
===========================================
  Files          276      276              
  Lines        17950    17950              
===========================================
- Hits         16003    16002       -1     
- Misses        1947     1948       +1

Impacted Files	Coverage Δ
core/base/extended_float.hpp	`92.30% <0.00%> (-1.10%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db2c566...1c9b579. Read the comment docs.

.gitlab-ci.yml

pratikvn

LGTM! In the final merge message, can you just list the changes/workarounds you had to make so that we have it all in one place ?

stale

tcojean

LGTM. Some small comments.

.gitlab-ci.yml

tcojean · 2020-05-25T15:07:18Z

examples/custom-matrix-format/CMakeLists.txt

@@ -8,4 +8,6 @@ if (GINKGO_BUILD_CUDA AND GINKGO_BUILD_OMP)
    target_link_libraries(custom-matrix-format ginkgo)
    target_include_directories(custom-matrix-format PRIVATE
        ${PROJECT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR})
+    # workaround for clang-cuda/g++ interaction
+    set_target_properties(custom-matrix-format PROPERTIES POSITION_INDEPENDENT_CODE ON)


Just a comment here, in general my understanding is that this makes the code more robust but could create some performance hit. Nonetheless, it feels like position independent code is in general a good idea. I always wondered whether we should make this a default or not. I don't think it's the best place to discuss it, but if anyone has a strong opinion I would like to hear it.

This is only an issue because for some reason, g++ expects relocatable code from the CUDA file, but clang-cuda generates static code. For shared/static libraries, I think CMake already takes care of the PIC property.

For that issue that's fine, what I meant was generally whether we should or not always put this flag on anyway.

I'm not really sure we need it, the only place where I actually had to manually set this was when generating an object library or an intermediate library that was being used to (statically) build a larger library. Do you have a situation in mind where this would be necessary?

also fix the settings of a mis-named job

g++ expects PIC code, clang-cuda only generates static

sonarqubecloud · 2020-05-25T23:53:03Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
No Duplication information

The Ginkgo team is proud to announce the new minor release of Ginkgo version 1.2.0. This release brings full HIP support to Ginkgo, new preconditioners (ParILUT, ISAI), conversion between double and float for all LinOps, and many more features and fixes. Supported systems and requirements: + For all platforms, cmake 3.9+ + Linux and MacOS + gcc: 5.3+, 6.3+, 7.3+, all versions after 8.1+ + clang: 3.9+ + Intel compiler: 2017+ + Apple LLVM: 8.0+ + CUDA module: CUDA 9.0+ + HIP module: ROCm 2.8+ + Windows + MinGW and CygWin: gcc 5.3+, 6.3+, 7.3+, all versions after 8.1+ + Microsoft Visual Studio: VS 2017 15.7+ + CUDA module: CUDA 9.0+, Microsoft Visual Studio + OpenMP module: MinGW or CygWin. The current known issues can be found in the [known issues page](https://github.com/ginkgo-project/ginkgo/wiki/Known-Issues). # Additions Here are the main additions to the Ginkgo library. Other thematic additions are listed below. + Add full HIP support to Ginkgo [#344](#344), [#357](#357), [#384](#384), [#373](#373), [#391](#391), [#396](#396), [#395](#395), [#393](#393), [#404](#404), [#439](#439), [#443](#443), [#567](#567) + Add a new ISAI preconditioner [#489](#489), [#502](#502), [#512](#512), [#508](#508), [#520](#520) + Add support for ParILUT and ParICT factorization with ILU preconditioners [#400](#400) + Add a new BiCG solver [#438](#438) + Add a new permutation matrix format [#352](#352), [#469](#469) + Add CSR SpGEMM support [#386](#386), [#398](#398), [#418](#418), [#457](#457) + Add CSR SpGEAM support [#556](#556) + Make all solvers and preconditioners transposable [#535](#535) + Add CsrBuilder and CooBuilder for intrusive access to matrix arrays [#437](#437) + Add a standard-compliant allocator based on the Executors [#504](#504) + Support conversions for all LinOp between double and float [#521](#521) + Add a new boolean to the CUDA and HIP executors to control DeviceReset (default off) [#557](#557) + Add a relaxation factor to IR to represent Richardson Relaxation [#574](#574) + Add two new stopping criteria, for relative (to `norm(b)`) and absolute residual norm [#577](#577) ### Example additions + Templatize all examples to simplify changing the precision [#513](#513) + Add a new adaptive precision block-Jacobi example [#507](#507) + Add a new IR example [#522](#522) + Add a new Mixed Precision Iterative Refinement example [#525](#525) + Add a new example on iterative trisolves in ILU preconditioning [#526](#526), [#536](#536), [#550](#550) ### Compilation and library changes + Auto-detect compilation settings based on environment [#435](#435), [#537](#537) + Add SONAME to shared libraries [#524](#524) + Add clang-cuda support [#543](#543) ### Other additions + Add sorting, searching and merging kernels for GPUs [#403](#403), [#428](#428), [#417](#417), [#455](#455) + Add `gko::as` support for smart pointers [#493](#493) + Add setters and getters for criterion factories [#527](#527) + Add a new method to check whether a solver uses `x` as an initial guess [#531](#531) + Add contribution guidelines [#549](#549) # Fixes ### Algorithms + Improve the classical CSR strategy's performance [#401](#401) + Improve the CSR automatical strategy [#407](#407), [#559](#559) + Memory, speed improvements to the ELL kernel [#411](#411) + Multiple improvements and fixes to ParILU [#419](#419), [#427](#427), [#429](#429), [#456](#456), [#544](#544) + Fix multiple issues with GMRES [#481](#481), [#523](#523), [#575](#575) + Optimize OpenMP matrix conversions [#505](#505) + Ensure the linearity of the ILU preconditioner [#506](#506) + Fix IR's use of the advanced apply [#522](#522) + Fix empty matrices conversions and add tests [#560](#560) ### Other core functionalities + Fix complex number support in our math header [#410](#410) + Fix CUDA compatibility of the main ginkgo header [#450](#450) + Fix isfinite issues [#465](#465) + Fix the Array::view memory leak and the array/view copy/move [#485](#485) + Fix typos preventing use of some interface functions [#496](#496) + Fix the `gko::dim` to abide to the C++ standard [#498](#498) + Simplify the executor copy interface [#516](#516) + Optimize intermediate storage for Composition [#540](#540) + Provide an initial guess for relevant Compositions [#561](#561) + Better management of nullptr as criterion [#562](#562) + Fix the norm calculations for complex support [#564](#564) ### CUDA and HIP specific + Use the return value of the atomic operations in our wrappers [#405](#405) + Improve the portability of warp lane masks [#422](#422) + Extract thread ID computation into a separate function [#464](#464) + Reorder kernel parameters for consistency [#474](#474) + Fix the use of `pragma unroll` in HIP [#492](#492) ### Other + Fix the Ginkgo CMake installation files [#414](#414), [#553](#553) + Fix the Windows compilation [#415](#415) + Always use demangled types in error messages [#434](#434), [#486](#486) + Add CUDA header dependency to appropriate tests [#452](#452) + Fix several sonarqube or compilation warnings [#453](#453), [#463](#463), [#532](#532), [#569](#569) + Add shuffle tests [#460](#460) + Fix MSVC C2398 error [#490](#490) + Fix missing interface tests in test install [#558](#558) # Tools and ecosystem ### Benchmarks + Add better norm support in the benchmarks [#377](#377) + Add CUDA 10.1 generic SpMV support in benchmarks [#468](#468), [#473](#473) + Add sparse library ILU in benchmarks [#487](#487) + Add overhead benchmarking capacities [#501](#501) + Allow benchmarking from a matrix list file [#503](#503) + Fix benchmarking issue with JSON and non-finite numbers [#514](#514) + Fix benchmark logger crashers with OpenMP [#565](#565) ### CI related + Improvements to the CI setup with HIP compilation [#421](#421), [#466](#466) + Add MacOSX CI support [#470](#470), [#488](#488) + Add Windows CI support [#471](#471), [#488](#488), [#510](#510), [#566](#566) + Use sanitizers instead of valgrind [#476](#476) + Add automatic container generation and update facilities [#499](#499) + Fix the CI parallelism settings [#517](#517), [#538](#538), [#539](#539) + Make the codecov patch check informational [#519](#519) + Add support for LLVM sanitizers with improved thread sanitizer support [#578](#578) ### Test suite + Add an assertion for sparsity pattern equality [#416](#416) + Add core and reference multiprecision tests support [#448](#448) + Speed up GPU tests by avoiding device reset [#467](#467) + Change test matrix location string [#494](#494) ### Other + Add Ginkgo badges from our tools [#413](#413) + Update the `create_new_algorithm.sh` script [#420](#420) + Bump copyright and improve license management [#436](#436), [#433](#433) + Set clang-format minimum requirement [#441](#441), [#484](#484) + Update git-cmake-format [#446](#446), [#484](#484) + Disable the development tools by default [#442](#442) + Add a script for automatic header formatting [#447](#447) + Add GDB pretty printer for `gko::Array` [#509](#509) + Improve compilation speed [#533](#533) + Add editorconfig support [#546](#546) + Add a compile-time check for header self-sufficiency [#552](#552) # Related PR: #583

upsj added the reg:build This is related to the build system. label May 22, 2020

upsj self-assigned this May 22, 2020

tcojean previously approved these changes May 22, 2020

View reviewed changes

upsj added this to the Ginkgo 1.2.0 milestone May 22, 2020

upsj requested review from yhmtsai, pratikvn and thoasm May 22, 2020 12:49

upsj added the 1:ST:ready-for-review This PR is ready for review label May 22, 2020

thoasm previously approved these changes May 22, 2020

View reviewed changes

yhmtsai previously approved these changes May 22, 2020

View reviewed changes

upsj force-pushed the clang_cuda_support branch from 630f767 to d3c258f Compare May 22, 2020 17:36

upsj added 1:ST:WIP This PR is a work in progress. Not ready for review. and removed 1:ST:ready-for-review This PR is ready for review labels May 22, 2020

upsj force-pushed the clang_cuda_support branch from 1733844 to db408de Compare May 23, 2020 08:45

upsj added 1:ST:ready-for-review This PR is ready for review and removed 1:ST:WIP This PR is a work in progress. Not ready for review. labels May 23, 2020

upsj requested review from thoasm, yhmtsai and tcojean May 23, 2020 12:34

pratikvn reviewed May 24, 2020

View reviewed changes

.gitlab-ci.yml Show resolved Hide resolved

pratikvn approved these changes May 24, 2020

View reviewed changes

upsj force-pushed the clang_cuda_support branch from 800d715 to af73293 Compare May 24, 2020 17:22

tcojean approved these changes May 25, 2020

View reviewed changes

fix clang-cuda arguments

96d6efc

upsj force-pushed the clang_cuda_support branch from af73293 to d8573b8 Compare May 25, 2020 15:44

upsj added 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:ready-for-review This PR is ready for review labels May 25, 2020

upsj added 5 commits May 25, 2020 23:52

add clang-cuda CI jobs

7261c70

also fix the settings of a mis-named job

fix linker issues with g++ and clang-cuda

3893ad5

g++ expects PIC code, clang-cuda only generates static

find Threads with installed Ginkgo if we built for HIP

2b63cd3

fix missing find_package for HIP LinOp benchmarks

6900104

update commit hashes and image names after merge

1c9b579

upsj force-pushed the clang_cuda_support branch from d8573b8 to 1c9b579 Compare May 25, 2020 21:53

upsj merged commit fcdc7e0 into develop May 26, 2020

upsj deleted the clang_cuda_support branch May 26, 2020 06:19

tcojean mentioned this pull request Jun 23, 2020

Release/1.2.0 #576

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for clang-cuda #543

Add support for clang-cuda #543

upsj commented May 22, 2020

tcojean left a comment

thoasm left a comment

upsj commented May 22, 2020 •

edited

Loading

upsj commented May 23, 2020

upsj commented May 23, 2020

codecov bot commented May 23, 2020 •

edited

Loading

pratikvn left a comment

tcojean left a comment

tcojean May 25, 2020

upsj May 25, 2020

tcojean May 25, 2020

upsj May 25, 2020

sonarqubecloud bot commented May 25, 2020

Add support for clang-cuda #543

Add support for clang-cuda #543

Conversation

upsj commented May 22, 2020

tcojean left a comment

Choose a reason for hiding this comment

thoasm left a comment

Choose a reason for hiding this comment

upsj commented May 22, 2020 • edited Loading

upsj commented May 23, 2020

upsj commented May 23, 2020

codecov bot commented May 23, 2020 • edited Loading

Codecov Report

pratikvn left a comment

Choose a reason for hiding this comment

tcojean left a comment

Choose a reason for hiding this comment

tcojean May 25, 2020

Choose a reason for hiding this comment

upsj May 25, 2020

Choose a reason for hiding this comment

tcojean May 25, 2020

Choose a reason for hiding this comment

upsj May 25, 2020

Choose a reason for hiding this comment

sonarqubecloud bot commented May 25, 2020

upsj commented May 22, 2020 •

edited

Loading

codecov bot commented May 23, 2020 •

edited

Loading