[PATCH] D151088: [flang][hlfir] Separate -emit-fir and -emit-hlfir for flang-new

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.


================
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.");
----------------
tblah wrote:
> awarzynski wrote:
> > 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`).
> This is very different to `EmitMLIR`.
> 
> `EmitMLIR` will print the MLIR produced by lowering (HLFIR or FIR depending upon the `-flang-experimental-hlfir` flag).
> 
> This action will run lowering, always generating HLFIR. Then it will run the HLFIR pass pipeline to lower the HLFIR into FIR, and output that FIR (which will be very different to FIR generated directly from lowering without going through HLFIR).
So, at the moment:
* `EmitMLIR` will generate either FIR of HLFIR,
* `lowerHLFIRToFIR` will also generate FIR via HFLIR.

When `-flang-experimental-hlfir` is used, both actions do the same thing (**what**) - generate FIR. The difference becomes in **how** this is achieved.  However, actions represent "what", not "how" and hence my suggestion. In fact,  this would be totally valid:
```
  /// (with `-flang-experimental-hlfir`) Lower to FIR and print
  EmitMLIR,

  /// Lower to FIR and print, go via HLFIR
  EmitHLFIRToFIR,
```
With `EmitFIR` and `EmitHLFIR` (instead of `EmitMLIR` and `EmitHLFIRToFIR`), you'd preserve all the functionality and also make sure that there's never overlap between actions, right? Additionally, you'd still have `-flang-experimental-hlfir` that would tweak `EmitFIR`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151088