This is an archive of the discontinued LLVM Phabricator instance.

Add GPU async op interface and token type.
ClosedPublic

Authored by csigg on Oct 7 2020, 3:57 AM.

Diff Detail

Event Timeline

csigg created this revision.Oct 7 2020, 3:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2020, 3:57 AM
csigg requested review of this revision.Oct 7 2020, 3:57 AM
herhut added a comment.Oct 7 2020, 9:50 AM

Can you add a printing/parsing roundtrip test for the type?

mlir/include/mlir/Dialect/GPU/GPUBase.td
57

Do we want a space in the type name? I think this triggers ugly printing with quotes.

64

Should this be ::mlir::gpu?

78

Would it be useful to have an addAsyncDependencies instead?

I don't quite understand yet the relationship between the "GPU async interface", the "gpu async token", and the async dialect?

csigg updated this revision to Diff 296919.Oct 8 2020, 4:22 AM

Reviewer comments.

csigg marked 3 inline comments as done.Oct 8 2020, 4:22 AM
csigg added a comment.Oct 8 2020, 4:47 AM

Can you add a printing/parsing roundtrip test for the type?

Done.

I don't quite understand yet the relationship between the "GPU async interface", the "gpu async token", and the async dialect?

The tokens are passed between ops implementing the async interface to model dependencies between them.
I hope the extra documentation in the latest revision helps.

csigg updated this revision to Diff 296926.Oct 8 2020, 4:47 AM

Add test for !gpu.async.token roundtripping.

herhut accepted this revision.Oct 9 2020, 4:54 AM
herhut added inline comments.
mlir/include/mlir/Dialect/GPU/GPUBase.td
91

Should this be ::mlir::gpu in case this interface is added to a dialect that is outside of mlir?

This revision is now accepted and ready to land.Oct 9 2020, 4:54 AM
csigg updated this revision to Diff 297314.Oct 9 2020, 12:56 PM

Fully qualify namespace.

csigg updated this revision to Diff 297315.Oct 9 2020, 12:58 PM

Rebase.

csigg marked an inline comment as done.Oct 9 2020, 12:58 PM
This revision was automatically updated to reflect the committed changes.