[PATCH] D85508: [flang] Add rudimentary empty function lowering to tco
Mehdi AMINI via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 10 18:03:23 PDT 2020
mehdi_amini added a comment.
We really need passes to be tested individually with an `opt` like tool, let me know if you need more pointers.
I suspect you also should invest up-front in the same registration/description for passes as in the mlir core folder.
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1
+#include "flang/Optimizer/CodeGen/CodeGen.h"
+#include "mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h"
----------------
There should be a license header here I believe?
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:18
+static cl::opt<bool> disableLLVM("disable-llvm", cl::desc("disable LLVM pass"),
+ cl::init(false), cl::Hidden);
+
----------------
This kind of option is unusual: if you want to disable passes this is better done at the point where the pass pipeline is built.
(Also please use PassOptions instead of globals)
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:26
+// Lower a SELECT operation into a cascade of conditional branches. The
+// last case must be the `true` condition.
+/// Convert FIR dialect to LLVM dialect
----------------
This comment seems misplaced
================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:62
+private:
+ fir::NameUniquer &uniquer;
+};
----------------
In general passes aren't supposed to depends on external entities like this: they are expected to be registered and invoked from an *-opt tool.
Here isn't clear why it can't just be a private instance for the pass?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85508/new/
https://reviews.llvm.org/D85508
More information about the llvm-commits
mailing list