[PATCH] D118985: [flang][driver] Add support for `-emit-mlir`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.


================
Comment at: flang/lib/Frontend/FrontendActions.cpp:69
+
+  // Create a LoweringBridge
+  auto &defKinds = ci.invocation().semanticsContext().defaultKinds();
----------------
kiranchandramohan wrote:
> Nit: Can we remove the three autos below?
Sure!


================
Comment at: flang/lib/Frontend/FrontendActions.cpp:419
+
+  // ... otherwise, print to a file.
+  auto os{ci.CreateDefaultOutputFile(
----------------
kiranchandramohan wrote:
> Nit: Should we test the existence of such a file?
We do :) Sort of!

To me, "existence" is a low level concept. Files are handled through e.g. `llvm::raw_fd_ostream` (and occasionally other streams) and IMO all low level details should be dealt with there (and indeed are). `flang-new` should only verify that `os` is not a `nullptr`. If the file does not exist, `os` will be a `nullptr` and that's checked further down. If the file does exist, then everything is fine and we can move to the next step. 


================
Comment at: flang/lib/Frontend/FrontendActions.cpp:420
+  // ... otherwise, print to a file.
+  auto os{ci.CreateDefaultOutputFile(
+      /*Binary=*/true, /*InFile=*/GetCurrentFileOrBufferName(), "mlir")};
----------------
kiranchandramohan wrote:
> Nit: can we remove auto?
Sure!


================
Comment at: flang/test/Driver/emit-mlir.f90:1
+! The the `-emit-fir` option
+
----------------
kiranchandramohan wrote:
> Nit: The the
Thanks :) Also, should be `-emit-mlir` instead of  `-emit-fir`.


================
Comment at: flang/test/Driver/syntax-only.f90:16
-! FSYNTAX_ONLY: IF statement is not allowed in IF statement
-! FSYNTAX_ONLY_NEXT: Semantic errors in {{.*}}syntax-only.f90
-
----------------
kiranchandramohan wrote:
> Nit: Do you have another test for `fsyntax-only`?
No, but there are many tests that use it: https://github.com/llvm/llvm-project/blob/main/flang/test/Semantics/call17.f90


================
Comment at: flang/test/Driver/syntax-only.f90:23
+! CHECK-NOT: error
 ! NO_FSYNTAX_ONLY: error: code-generation is not available yet
 
----------------
kiranchandramohan wrote:
> Do we currently run the stages before codegen and then issue this error?
Yes, but this error is here only temporarily. I will be removing it shortly (once `-emit-obj` is implemented).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118985