This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add gpu.allocate, gpu.deallocate ops with LLVM lowering to runtime function calls.
ClosedPublic

Authored by csigg on Nov 18 2020, 4:26 AM.

Details

Summary

The ops are very similar to the std variants, but support async GPU execution.

gpu.alloc does not currently support an alignment attribute, and the new ops do not have
canonicalizers/folders like their std siblings do.

Diff Detail

Event Timeline

csigg created this revision.Nov 18 2020, 4:26 AM
csigg requested review of this revision.Nov 18 2020, 4:26 AM
herhut requested changes to this revision.Nov 23 2020, 6:36 AM
herhut added inline comments.
mlir/include/mlir/Dialect/GPU/GPUOps.td
810

Not for now but I was wondering whether we should have a different resource for GPU allocations? Maybe also to use as a key for buffer assignment to insert the corresponding free.

mlir/lib/Conversion/GPUCommon/ConvertLaunchFuncToRuntimeCalls.cpp
180

These should implement the public matchAndRewrite(gpu::AllocOp op, ArrayRef<Value> operands, ConversionPatternRewriter &rewriter instead of overriding the private one. Just noticed that all patterns currently do the latter.

345

Use the gpu::AllocOp typed version.

350

Instead of match, would it be more natural to use is or have or something?

658–662

nit: sort.

mlir/test/Conversion/GPUCommon/lower-alloc-to-gpu-runtime-calls.mlir
7

Maybe test with a bit more context to also capture that streams are passed correctly? I think it is ok to ignore the memref construction part, though.

This revision now requires changes to proceed.Nov 23 2020, 6:36 AM
csigg updated this revision to Diff 307439.Nov 24 2020, 1:08 PM
csigg marked 2 inline comments as done.

Thanks for the review!

csigg marked an inline comment as done.Nov 24 2020, 1:09 PM
csigg added inline comments.
mlir/include/mlir/Dialect/GPU/GPUOps.td
810

Ack, makes sense. I can look into it.

mlir/lib/Conversion/GPUCommon/ConvertLaunchFuncToRuntimeCalls.cpp
180

That overload does not currently exist. I can add it to ConvertOpToLLVMPattern in a separate change and make this one final, just like in OpRewritePattern.

345

See comment above.

herhut accepted this revision.Nov 26 2020, 5:04 AM
This revision is now accepted and ready to land.Nov 26 2020, 5:04 AM