awarzynski added a comment.
Thanks Tom, mostly makes sense, but I suggest the following:
- `EmitMLIR` --> `EmitFIR` (nfc, just clarifying the name),
- `EmitHLFIRToFIR` --> `EmitHLFIR` (functional change).
More comments inline.
================
Comment at: flang/include/flang/Frontend/FrontendOptions.h:42
+ /// resulting FIR
+ EmitHLFIRToFIR,
+
----------------
To me having `EmitFIR` and `EmitHLFIR` would make more sense. With 2 dialects,
`EmitMLIR` becomes rather confusing (and, I suspect, rarely used).
================
Comment at: flang/lib/Frontend/FrontendActions.cpp:651
+// Lower using HLFIR then run the FIR to HLFIR pipeline
+void CodeGenAction::lowerHLFIRToFIR() {
+ assert(mlirModule && "The MLIR module has not been generated yet.");
----------------
I wouldn't really consider this hook as a separate action. Instead, I'd use it
here:
https://github.com/llvm/llvm-project/blob/6130c9df99a7a7eb9c6adc118a48f8f2acc534ab/flang/lib/Frontend/FrontendActions.cpp#L917-L920.
As in, it basically "tweaks" `EmitMLIR` (which I would rename as `EmitFIR`).
================
Comment at: flang/lib/Frontend/FrontendActions.cpp:652
+void CodeGenAction::lowerHLFIRToFIR() {
+ assert(mlirModule && "The MLIR module has not been generated yet.");
+
----------------
This `mlirModule` comes from here:
https://github.com/llvm/llvm-project/blob/6130c9df99a7a7eb9c6adc118a48f8f2acc534ab/flang/lib/Frontend/FrontendActions.cpp#L277.
That will either be FIR or HLFIR, depending on whether
`-flang-experimental-hlfir` was used or not, right?
================
Comment at: flang/lib/Frontend/FrontendActions.cpp:673
+ unsigned diagID = ci.getDiagnostics().getCustomDiagID(
+ clang::DiagnosticsEngine::Error, "Lowering to LLVM IR failed");
+ ci.getDiagnostics().Report(diagID);
----------------
"Lowering to LLVM IR"? ;-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151088/new/
https://reviews.llvm.org/D151088
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
