[PATCH] D156889: [mlir][cf] Add ControlFlow to SCF lifting pass

Tobias Gysi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 3 10:16:50 PDT 2023


gysit added inline comments.


================
Comment at: mlir/include/mlir/Transforms/CFGToSCF.h:37
+  /// left as is.
+  virtual Operation *createBranchRegionOp(OpBuilder &builder,
+                                          Operation *controlFlowCondOp,
----------------
Mogball wrote:
> gysit wrote:
> > Would it make sense to prefix all methods that create SCF operation with SCF? E.g., for this one it may make sense to rename to `createSCFBranchOp` and below `createSCFLoopOp`. That way it may be a bit easier to follow in the code when new SCF stuff is created.
> This is a generic interface and isn't specific to the SCF dialect (although the name of the interface is confusing SCF as in structured control flow with SCF the dialect...)
I agree this is a bit confusing - in most places in this revision SCF actually refers to structured control flow an not to the dialect. Any suggestions for an alternative naming that does not collide with the dialect name is very welcome. Maybe StructuredCF?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156889



More information about the llvm-commits mailing list