[PATCH] D75837: Introduce std.execute_region op

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache added a comment.

Responding in bulk below:

> @nicolasvasilache is that ok with you?

I am not opposed to moving forward and iterating on code as we learn more.
But I still haven't seen a compelling reason to disallow arguments.
I would like some concrete example that illustrates why allowing arguments is a bad idea.

> We can loosen it later if there's a compelling need.

This seems at odds with this other statement:

> Keeping arguments around will necessitate all kinds of tracking/bookkeeping in moving code across, reimplementing existing canonicalizations on this op and largely defeating the purpose of this op - which is to let SSA dominance and dataflow work freely from above and through it.

In other words, either:

1. there are fundamental difficulties involved in which case, refusing arguments pushes concerns to all consumers. Shouldn't difficulty be factored out in one place
2. or it's a simple extension, in which case why not just allow arguments?

I am unclear if we are in case 1., 2. or something else. Which is it?

> @mehdi_amini Can you provide an example of what you?

See `linalg.generic` and `linalg.indexed_generic`, both have region arguments that are derived from the op operands, but are not necessarily the same SSA value 
(e.g there is an interleaved load/store or even loop IV creation).

Now I can see how to use this op in its current form for my particular purpose: I can just move the content of my region inside a new execute_region op as I lower.

But I don't think my questions have been answered so I'll ask again:

  Lambdas often allow both captures and arguments.
  ...
  Is there a fundamental reason to disallow arguments in your op?
  Assuming there exists such a reason, isn't it trivial to check preconditions such as "empty arguments" in verifiers that need it?




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75837/new/

https://reviews.llvm.org/D75837