[PATCH] D77870: [MLIR] Introduce utility to hoist affine if/else conditions

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/include/mlir/Dialect/Affine/Utils.h:24
+/// hoisting could lead to significant code expansion in some cases.
+bool hoistAffineIfOp(AffineIfOp ifOp, bool *folded = nullptr);
+
----------------
bondhugula wrote:
> rriddle wrote:
> > Why not LogicalResult?
> It would always return success() then - this utility can't fail. (I didn't think 'no hoisting' should be called a failure, but if we do, we could use LogicalResult.)
I would say that success/failure takes on different meanings depending on the context. Here I would read `success` as the if was successfully hoisted. This is different from the failure mode that was decided on for passes.  e.g., the folding methods returns success if the operation was folded, and failure otherwise.   


================
Comment at: mlir/include/mlir/IR/Operation.h:132
+  Operation *getParentWithTrait() {
+    auto *op = this;
+    while ((op = op->getParentOp()))
----------------
nit: auto -> Operation


================
Comment at: mlir/lib/Dialect/Affine/Utils/Utils.cpp:140
+  OwningRewritePatternList patterns;
+  ifOp.getCanonicalizationPatterns(patterns, ifOp.getContext());
+  bool erased;
----------------
Please don't call static functions as members.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77870