[PATCH] D119012: [flang][driver] Add support for the `-emit-llvm` option

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

The tests added are failing in the windows CI.



================
Comment at: flang/lib/Frontend/FrontendActions.cpp:421
+  // Set-up the MLIR pass manager
+  fir::setTargetTriple(*mlirModule_, "native");
+  auto &defKinds = ci.invocation().semanticsContext().defaultKinds();
----------------
awarzynski wrote:
> rovka wrote:
> > Nit: Should we assert that mlirModule exists?
> > Also, why doesn't it already have a triple and a kind mapping?
> > Nit: Should we assert that mlirModule exists?
> 
> We could do, yes. However, `mlirModule` is created in `CodeGenAction::BeginSourceFileAction`. If that method fails, the driver should stop immediately. So perhaps that would be a better for place for an assert?
> 
> On a related note, `mlirModule` is obtained via [[ https://github.com/llvm/llvm-project/blob/79b3fe80707b2eb9a38c1517a829fb58062fb687/flang/include/flang/Lower/Bridge.h#L67 | LoweringBridge::getModule ]]. But if the corresponding module is `nullptr` than that getter should probably assert. See https://reviews.llvm.org/D119133.
> 
> >  Also, why doesn't it already have a triple and a kind mapping?
> Good catch, this is not needed yet. I didn't notice this when going over `tco`.
 D118985 creates a bridge with an empty triple. The patch here was switching it to "native". Please cross-check what the expected behaviour.

```
  lower::LoweringBridge lb = Fortran::lower::LoweringBridge::create(*mlirCtx,
      defKinds, ci.invocation().semanticsContext().intrinsics(),
      ci.parsing().allCooked(), "", kindMap);
```


================
Comment at: flang/lib/Frontend/FrontendActions.cpp:440
+  // Translate to LLVM IR
+  auto optName = mlirModule->getName();
+  llvmCtx = std::make_unique<llvm::LLVMContext>();
----------------
Nit: Remove auto.


================
Comment at: flang/unittests/Frontend/FrontendActionTest.cpp:176
+  // stream, as opposed to an actual file (or a file descriptor).
+  llvm::SmallVector<char, 256> outputFileBuffer;
+  std::unique_ptr<llvm::raw_pwrite_stream> outputFileStream(
----------------
Nit: Is the size important?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119012