This is an archive of the discontinued LLVM Phabricator instance.

[mlir][gpu][sparse] add gpu ops for sparse matrix computations
ClosedPublic

Authored by aartbik on May 8 2023, 4:19 PM.

Details

Summary

This revision extends the GPU dialect with ops that can be lowered to
host-oriented sparse matrix library calls (in this case cuSparse focused
although the ops could be generalized to support more GPUs in principle).
This will allow the "sparse compiler pipeline" to accelerate sparse operations
(see follow up revisions with examples of this).

For some background;

https://discourse.llvm.org/t/sparse-compiler-and-gpu-code-generation/69786/2

Diff Detail

Event Timeline

aartbik created this revision.May 8 2023, 4:19 PM
Herald added a reviewer: dcaballe. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
aartbik requested review of this revision.May 8 2023, 4:19 PM
aartbik added inline comments.May 8 2023, 4:25 PM
utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
7760

I could use some help making sure that

lib/ExecutionEngine/CMakeLists.txt

also keeps working with cusparse.h dependence.

aartbik added a reviewer: K-Wu.May 8 2023, 6:28 PM
aartbik updated this revision to Diff 520549.May 8 2023, 6:32 PM

removed debugging code

aartbik retitled this revision from [gpu][sparse] add gpu ops for sparse matrix computations to [mlir][gpu][sparse] add gpu ops for sparse matrix computations.May 8 2023, 7:09 PM
Matt added a subscriber: Matt.May 8 2023, 10:58 PM
aartbik updated this revision to Diff 520800.May 9 2023, 1:11 PM

rebased with main

aartbik updated this revision to Diff 520877.May 9 2023, 6:16 PM

cusparse build

aartbik added inline comments.May 10 2023, 9:32 AM
mlir/lib/ExecutionEngine/CMakeLists.txt
194 ↗(On Diff #520877)

A question for the reviewers is of course if this is an acceptable dependence (cuSPARSE has been part of CUDA for a long time now) and this is only pulled in when built with the extra flag MLIR_ENABLE_CUDA_RUNNER

aartbik updated this revision to Diff 521108.May 10 2023, 2:40 PM

rebased with main

aartbik edited the summary of this revision. (Show Details)May 11 2023, 10:21 AM
aartbik updated this revision to Diff 521435.May 11 2023, 1:58 PM

rebased with main

aartbik updated this revision to Diff 521439.May 11 2023, 2:03 PM

minor edit

aartbik updated this revision to Diff 521444.May 11 2023, 2:09 PM

minor edit

ThomasRaoux accepted this revision.May 12 2023, 6:59 AM

LGTM

mlir/lib/ExecutionEngine/CMakeLists.txt
194 ↗(On Diff #520877)

That looks reasonable to me, an alternative would be to introduce another flag but if we expect cuSPARSE to be there along with CUDA runtime I don't see a reason for that.

mlir/lib/ExecutionEngine/CudaRuntimeWrappers.cpp
226

nit: remove empty line?

This revision is now accepted and ready to land.May 12 2023, 6:59 AM
aartbik marked 2 inline comments as done.May 12 2023, 9:42 AM
aartbik added inline comments.
mlir/lib/ExecutionEngine/CMakeLists.txt
194 ↗(On Diff #520877)

Thanks for the pointer! Yeah, if this turns out to be a breaker, I will add the flag, but for now let's proceed with the assumption.

mlir/lib/ExecutionEngine/CudaRuntimeWrappers.cpp
226

I followed the style found at L161 on introducing a new section. But I made it a bit more clear by adding extra /// on such lines.

aartbik updated this revision to Diff 521692.May 12 2023, 9:44 AM
aartbik marked 2 inline comments as done.

addressed comments