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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
