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