Page MenuHomePhabricator

[mlir:Async] Remove async operations if it is statically known that the parallel operation has a single compute block
ClosedPublic

Authored by ezhulenev on Jun 24 2021, 5:48 PM.

Details

Summary

Depends On D104850

Add a test that verifies that canonicalization removes all async overheads if it is statically known that the scf.parallel operation will be computed using a single block.

Diff Detail

Event Timeline

ezhulenev created this revision.Jun 24 2021, 5:48 PM
ezhulenev requested review of this revision.Jun 24 2021, 5:48 PM

Add more checks to the test

ezhulenev edited the summary of this revision. (Show Details)Jun 25 2021, 10:16 AM
ezhulenev added a reviewer: herhut.

Revert accidental changes

bondhugula added inline comments.
mlir/lib/Dialect/Async/IR/Async.cpp
252–275

This can be in the op's folding hook as opposed to a canonicalization pattern.

utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
1516–1525 ↗(On Diff #354771)

Any reason this is moved up? It's no longer in alphabetical order.

ezhulenev updated this revision to Diff 354780.Sun, Jun 27, 5:44 PM
ezhulenev marked an inline comment as done.

Remove irrelevant files

mlir/lib/Dialect/Async/IR/Async.cpp
252–275

Can it? I think folding hook can only return an attribute, but it doesn't have access to rewriter to erase operations.

utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
1516–1525 ↗(On Diff #354771)

That's the result of out internal automation tool, I removed these changes from this PR.

bondhugula added inline comments.Sun, Jun 27, 5:57 PM
mlir/lib/Dialect/Async/IR/Async.cpp
252–275

Sorry, I had missed the erasure of AwaitAllOps. The folding hook can't of course be used to erase other ops.

mehdi_amini added inline comments.Sun, Jun 27, 6:37 PM
mlir/lib/Dialect/Async/IR/Async.cpp
252–275

The folding hook can return an attribute or an existing SSA value in the IR, it shouldn't create new IR.
This value will be used to replace the existing result, and the original op will be deleted by the folder.

ezhulenev updated this revision to Diff 355028.Mon, Jun 28, 1:54 PM

Drop reference if the value has no uses

herhut accepted this revision.Tue, Jun 29, 8:43 AM
herhut added inline comments.
mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp
527

Nit: I think the comment about async groups is hard to understand outside the scope of this review, where one sees all this in one place. It is also not very relevant here.

mlir/test/Dialect/Async/async-parallel-for-canonicalize.mlir
14

nit: synchronous.
2nd nit: There is no function call anymore. Maybe also mention in the comment that we have a single call that gets inlined.

This revision is now accepted and ready to land.Tue, Jun 29, 8:43 AM