This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Lower async.func with async.coro and async.runtime operations
ClosedPublic

Authored by yijia1212 on Nov 4 2022, 1:18 PM.

Details

Summary

Lower async.func with async.coro and async.runtime operations

  • This patch modifies AsyncToAsyncRuntime pass to add lowering async.func ops with coroutine cfg.

Example:

async.func @foo() -> !async.value<f32> {
  %cst = arith.constant 42.0 : f32
  return %cst: f32
}

After lowering:

func.func @foo() -> !async.value<f32> attributes {passthrough = ["presplitcoroutine"]} {
    %0 = async.runtime.create : !async.value<f32>
    %1 = async.coro.id
    %2 = async.coro.begin %1
    cf.br ^bb1
  ^bb1:  // pred: ^bb0
    %cst = arith.constant 4.200000e+01 : f32
    async.runtime.store %cst, %0 : <f32>
    async.runtime.set_available %0 : !async.value<f32>
    cf.br ^bb2
  ^bb2:  // pred: ^bb1
    async.coro.free %1, %2
    cf.br ^bb3
  ^bb3:  // pred: ^bb2
    async.coro.end %2
    return %0 : !async.value<f32>
}

Diff Detail

Event Timeline

yijia1212 created this revision.Nov 4 2022, 1:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 4 2022, 1:18 PM
yijia1212 requested review of this revision.Nov 4 2022, 1:18 PM
yijia1212 edited the summary of this revision. (Show Details)Nov 4 2022, 1:20 PM
yijia1212 added a reviewer: ezhulenev.
yijia1212 updated this revision to Diff 473332.Nov 4 2022, 1:57 PM

Fix test error

ezhulenev added inline comments.Nov 5 2022, 8:26 PM
mlir/lib/Dialect/Async/Transforms/AsyncToAsyncRuntime.cpp
66

mega-nit: use T here for consistency with execute, or use f32 later

77

nit: move comment before the variable declaration to keep it on a single line, or rewrite it to make a bit shorter (returned completion token?)

82

setError is also optional. Given that Value is smart-pointer-like and default constructed Value is like null optional, for consistency it should be either Optional<Block*> setError or Value asyncToken (you can just use if (asyncToken) to check if it's not-null).

I'd probably go for Optional in both cases to make it explicit in type: Optional<Block*> setError; // set returned values to error state

144

Use // inside function body, only top level function documentation uses ///

150

retToken.emplace(builder.create(...), then you'll be able to drop getResult

192

if (retToken) ret.push_back(*retToken);

230
408

nit: in every other patter op argument called op

488

ditto: bool & *

490

ditto: loc

677

ditto: operator bool and operator*

679

op->getLoc() -> loc

742

I don't remember why exactly async.execute outlining was done separately, I don't think there are reasons for doing it this way. Not for this PR, but it should also become a rewriter pattern like AyncFunc lowering.

yijia1212 updated this revision to Diff 473555.Nov 6 2022, 9:35 PM

Address comments

yijia1212 updated this revision to Diff 473556.Nov 6 2022, 9:41 PM
yijia1212 marked 9 inline comments as done.

Using operator* instead of optional.value()

yijia1212 marked 3 inline comments as done.Nov 6 2022, 9:42 PM
ezhulenev accepted this revision.Nov 7 2022, 9:47 AM
This revision is now accepted and ready to land.Nov 7 2022, 9:47 AM