Introduce std.inlined_call op - proposal

MLIR
21

+1. I don’t foresee this being inlined with the inliner pass, it would require some hacky special casing for a call to refer to itself as a callee. I imagine that we could reuse the same base utility to perform the actual inlining though.

I’m +1 on execute_region.

22

Sorry for the delay, past few days have been crazy…
Some pass by comments as I unpack the back and forths.

Not having an abstraction like this would mean that one is unable …

Could you please rephrase the triple negative?

because loop.for can only hold a single block while the callee could have multiple blocks.

Aren’t you overfitting on implementation details? It’s pretty easy to allow multiple regions in loop.for. In fact this structured control flow dialect is meant for such evolutions.

Note: I see this is proposed a bit later with also a canning proposal to “Region” instead of scf. This seems fine to me.

On the proposal itself: while I agree with the proposal and I am generally supportive of it I question the lack of arguments. Very concretely, to be generally useful for Linalg and other ops with regions that refuse to introduce SSA values prematurely (I.e. that use type information to encode the semantics and delay SSA value creation until inlining) you need both arguments and capture.

Can this be designed and implemented so it serves today’s needs that are already more general than “just capture”?

Thanks!

23

Maybe we could just expose a conversion pattern that folks can insert into their set of legalization conversion patterns?

24

This sounds good to start with, but we’ll also need something on the command line that lowers away this op. I’ll have a first patch that introduces this op. The second one will deal with the lowering.

25

For reference, here is the patch that introduces the op:
https://reviews.llvm.org/D75837

26

I really really like this op and want to see this committed. Let’s try to decouple it from the std.return discussion.

27

I’d like to dust this proposal and revision. There was consensus in favor of this op and the revision was also approved here: ⚙ D75837 [MLIR] Introduce scf.execute_region op. However, there were a couple of related things that were moving and that we can resolve now:

  1. This was earlier planned for the std dialect but post its split and trim, this would naturally fit into the scf dialect. So this will become scf.execute_region. The op’s concept is fully tied to a region and structured control flow.
  2. There was discussion around its terminator but that is now moot given that the scf.yield op in the scf dialect will naturally be the terminator for this op.

I plan to update and rebase the already approved PR later today: ⚙ D75837 [MLIR] Introduce scf.execute_region op It’ll be great if the reviewers could take another look.
@River707 @mehdi_amini @_sean_silva

28

Thanks Uday!