[PATCH] D95643: [flang][fir] Add structured FIR op translation pass.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: flang/lib/Optimizer/Transforms/PassDetail.h:18
+#define GEN_PASS_CLASSES
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+
----------------
I suspect we're missing the TableGen code to generate this file in this patch?


================
Comment at: flang/lib/Optimizer/Transforms/RewriteLoop.cpp:29
+    llvm::cl::desc("force the body of a loop to execute at least once"),
+    llvm::cl::init(false));
+
----------------
Can you make these "pass options"? https://mlir.llvm.org/docs/PassManagement/#instance-specific-pass-options
In general MLIR Passes can be free of global variables, which enables generating standalone reproducer on failures.


================
Comment at: flang/lib/Optimizer/Transforms/RewriteLoop.cpp:40
+
+/// Convert `fir.do_loop` to CFG
+class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
----------------
Could you lower to scf.for and rely on existing scf->CFG conversion?

Same question for the `if` below.


================
Comment at: flang/test/Fir/loop01.fir:1
+// RUN: tco %s | FileCheck %s
+
----------------
Is `tco` the actual `fir-opt` tool? (why not adopting the standard naming by the way?).

I'd expect an option with pass pipeline (or a pass invocation) to be specified on the command line here: the test should run only a single pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95643