[PATCH] D75837: Introduce std.execute_region op

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula added a comment.

> 
> 
>> 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?

It's not a simple extension. There are major costs. Straightforward SSA dominance vs having to pass through arguments (explicit captures) is akin to "intraprocedural optimization" vs "a good part of the complexity involved in interprocedural optimization" -- the latter has been established to be more difficult than intraprocedural for the same given transformation.

> Is there a fundamental reason to disallow arguments in your op?

Yes, I'm going to copy paste the same thing from above, but with a few extra lines below - I'm not sure if you had read this differently because the answer is pretty straightforward. "Keeping arguments around will necessitate all kinds of tracking/bookkeeping in moving code across the region boundary, 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."  You'd have to reimplement nearly all canonicalizations on this op from propagation of constants, to propagation of memref_casts, removal of dead deallocs, removal of dead allocs, subexpression elimination, etc.. For an example on just memref arguments, see the link upthread on grayboxes on the kind of complexities you'd have to deal with if you explicitly captured memrefs (over there IMO explicit captures just for memrefs are worth those cost and hence a new op affine.graybox). Did you skip reading the in-between messages?

To conclude, I just don't see the benefits of explicit captures in a few specific cases to outweigh the widespread / large scale negative impact on all lower level SSA optimizations (where low-level here is std dialect, loop dialect, and to some extent also the affine dialect - you'd have execute_region in the presence of these dialect ops *at least*) .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75837