[PATCH] D78447: [mlir][SCCP] Add support for propagating constants across inter-region control flow

[River Riddle via Phabricator via llvm-commits]

rriddle marked 4 inline comments as done.
rriddle added inline comments.


================
Comment at: mlir/lib/Dialect/LoopOps/LoopOps.cpp:357
+    regions.push_back(RegionSuccessor(&thenRegion()));
+    regions.push_back(RegionSuccessor(&elseRegion()));
+    return;
----------------
bondhugula wrote:
> What if there is no else block here? (empty else region)
I was originally undecided on how we should treat empty regions. For now, updated to disallow using empty regions as successors.


================
Comment at: mlir/test/Transforms/sccp-structured.mlir:132
+  return %result : i32
+}
----------------
bondhugula wrote:
> bondhugula wrote:
> > Every loop.if that you have in the test cases has an else. Add one or more without an else block?
> Just realized that no values would be yielded without the else. And you are testing for empty regions at one place. 
Do you have a suggestion on what you have in mind? An IfOp can only produce results if it has both a then and an else region. Given that IfOp is implicit capture, nothing would really be testing the structure of the IfOp that isn't tested elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78447