[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

[Sergio Afonso via Phabricator via cfe-commits]

skatrak added a comment.

Thanks again for helping improving this patch. I made most of the changes requested, and tried to clarify a bit better the approach followed for this implementation, to see if it is acceptable or if some more fundamental changes have to be done.



================
Comment at: flang/lib/Frontend/FrontendActions.cpp:103
+  std::error_code ec;
+  llvm::ToolOutputFile out(outDir + outFile, ec, llvm::sys::fs::OF_Text);
+  if (ec)
----------------
awarzynski wrote:
> Why not just [[ https://github.com/llvm/llvm-project/blob/3b1951aceb8c58acd3d5c5ba2d042ad3d03c13c4/flang/lib/Frontend/FrontendActions.cpp#L793 | print ]] the MLIR module?
I'm not sure I completely understand the issue here, by looking at the code you linked to. Maybe it's related to the fact that this function is implemented by creating new files apart from the main output of the current compiler invocation. I developed the idea a bit more in the reply to your comment on "save-temps.f90".


================
Comment at: flang/test/Driver/save-temps.f90:14
 ! CHECK-NEXT: "-o" "save-temps.o"
 ! CHECK-NEXT: "-o" "a.out"
 
----------------
awarzynski wrote:
> Why are there no MLIR files here? Same comment for other invocations.
This is because the general way in which -save-temps works is different from what's implemented in this patch for MLIR in flang. In the other cases, the driver splits the work into several frontend invocations, where each step generally produces the input of the next. `-save-temps` makes sure these intermediate files are kept where the user specified and not deleted.

In this patch, instead of modifying the driver to create a frontend invocation to produce MLIR (generating the *-fir.mlir file), another one to optimize/lower that (generating the *-llvmir.mlir file), and a third one to translate lowered MLIR into LLVM IR, we just forward the flag to the frontend, which creates extra MLIR files at particular spots of the codegen process if the flag is set. Hence, MLIR files don't show in the output of `flang-new -###`.


================
Comment at: flang/test/Driver/save-temps.f90:61
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: %flang_fc1 -emit-llvm-bc -save-temps=obj -o %t/out.bc %s 2>&1
+! RUN: FileCheck %s -input-file=%t/save-temps-FIR.mlir -check-prefix=MLIR-FIR
----------------
awarzynski wrote:
> What happens in this case: `-save-temps`? (no dir is specified)
The same as if `-save-temps=cwd` is specified. The test is now updated to check all supported options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146075