[PATCH] D78447: [mlir][SCCP] Add support for propagating constants in the presence of region based control flow.

[River Riddle via Phabricator via llvm-commits]

rriddle marked an inline comment as done.
rriddle added inline comments.


================
Comment at: mlir/lib/Transforms/SCCP.cpp:526-528
+    // If this terminator is not "region-like", conservatively mark all of the
+    // successor values as overdefined.
+    if (!op->hasTrait<OpTrait::ReturnLike>()) {
----------------
rriddle wrote:
> bondhugula wrote:
> > You missed marking `ReturnOp` `ReturnLike`.  As such, you will return early here and miss propagating lattice states to successor regions. Although there is no registered op that will exercise this, this could happen let's say in a special func-like op that has two regions (sort of two functions) and the semantics of the op were to execute the two regions one after the other. 
> Such an op isn't possible ATM given the constraints of ReturnOp. I intend to add interprocedural support here, at which point having it on ReturnOp would be useful. Also as a policy I like to avoid trying to mark every possible op that could take advantage of something. IMO it is better to separate adding a feature, and using it in every possible place. It keeps this patch more focused.
To clarify, I do intend to add this trait/interface to the things that can use it in a followup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78447