[Mlir-commits] [mlir] [flang] [flang] Fix seg fault `CodeGenAction::executeAction()` (PR #78551)
Kareem Ergawy
llvmlistbot at llvm.org
Thu Jan 18 00:55:25 PST 2024
https://github.com/ergawy created https://github.com/llvm/llvm-project/pull/78551
If `generateLLVMIR()` fails, we still continue using the module we
failed to generate which causes a seg fault if LLVM code-gen failed for
some reason or another. This commit fixes this issue.
This is a follow-up to https://github.com/llvm/llvm-project/pull/78269, please only review the latest commit.
>From 023815c470218338ac1c5dc7961bec5363af7b45 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Tue, 16 Jan 2024 06:56:23 -0600
Subject: [PATCH 1/2] [flang] Fix seg fault `CodeGenAction::executeAction()`
If `generateLLVMIR()` fails, we still continue using the module we
failed to generate which causes a seg fault if LLVM code-gen failed for
some reason or another. This commit fixes this issue.
---
flang/lib/Frontend/FrontendActions.cpp | 5 +
flang/unittests/Frontend/CMakeLists.txt | 1 +
.../unittests/Frontend/CodeGenActionTest.cpp | 109 ++++++++++++++++++
3 files changed, 115 insertions(+)
create mode 100644 flang/unittests/Frontend/CodeGenActionTest.cpp
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 74e3992d5ab62b..65c4df7388f97b 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -1202,6 +1202,11 @@ void CodeGenAction::executeAction() {
if (!llvmModule)
generateLLVMIR();
+ // If generating the LLVM module failed, abort! No need for further error
+ // reporting since generateLLVMIR() does this already.
+ if (!llvmModule)
+ return;
+
// Set the triple based on the targetmachine (this comes compiler invocation
// and the command-line target option if specified, or the default if not
// given on the command-line).
diff --git a/flang/unittests/Frontend/CMakeLists.txt b/flang/unittests/Frontend/CMakeLists.txt
index 79a394f161ed1e..3bcc37bed7f6d1 100644
--- a/flang/unittests/Frontend/CMakeLists.txt
+++ b/flang/unittests/Frontend/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
)
add_flang_unittest(FlangFrontendTests
+ CodeGenActionTest.cpp
CompilerInstanceTest.cpp
FrontendActionTest.cpp
)
diff --git a/flang/unittests/Frontend/CodeGenActionTest.cpp b/flang/unittests/Frontend/CodeGenActionTest.cpp
new file mode 100644
index 00000000000000..9d798c7678ad15
--- /dev/null
+++ b/flang/unittests/Frontend/CodeGenActionTest.cpp
@@ -0,0 +1,109 @@
+//===- unittests/Frontend/CodeGenActionTest.cpp --- FrontendAction tests --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Unit tests for CodeGenAction.
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/IR/Builders.h"
+#include "flang/Frontend/CompilerInstance.h"
+#include "flang/Frontend/FrontendActions.h"
+#include "flang/Frontend/TextDiagnosticPrinter.h"
+
+#include "gtest/gtest.h"
+
+#include <memory>
+
+using namespace Fortran::frontend;
+
+namespace test {
+class DummyDialect : public ::mlir::Dialect {
+ explicit DummyDialect(::mlir::MLIRContext *context)
+ : ::mlir::Dialect(getDialectNamespace(), context,
+ ::mlir::TypeID::get<DummyDialect>()) {
+ initialize();
+ }
+
+ void initialize();
+ friend class ::mlir::MLIRContext;
+
+public:
+ ~DummyDialect() override = default;
+ static constexpr ::llvm::StringLiteral getDialectNamespace() {
+ return ::llvm::StringLiteral("dummy");
+ }
+};
+
+namespace dummy {
+class FakeOp : public ::mlir::Op<FakeOp> {
+public:
+ using Op::Op;
+
+ static llvm::StringRef getOperationName() { return "dummy.fake"; }
+
+ static ::llvm::ArrayRef<::llvm::StringRef> getAttributeNames() { return {}; }
+
+ static void build(
+ ::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState) {}
+};
+} // namespace dummy
+} // namespace test
+
+MLIR_DECLARE_EXPLICIT_TYPE_ID(::test::DummyDialect)
+MLIR_DEFINE_EXPLICIT_TYPE_ID(::test::DummyDialect)
+
+namespace test {
+
+void DummyDialect::initialize() { addOperations<::test::dummy::FakeOp>(); }
+} // namespace test
+
+// A test CodeGenAction to verify that we gracefully handle failure to convert
+// from MLIR to LLVM IR.
+class LLVMConversionFailureCodeGenAction : public CodeGenAction {
+public:
+ LLVMConversionFailureCodeGenAction()
+ : CodeGenAction(BackendActionTy::Backend_EmitLL) {
+ mlirCtx = std::make_unique<mlir::MLIRContext>();
+ mlirCtx->loadDialect<test::DummyDialect>();
+
+ mlir::Location loc(mlir::UnknownLoc::get(mlirCtx.get()));
+ mlirModule =
+ std::make_unique<mlir::ModuleOp>(mlir::ModuleOp::create(loc, "mod"));
+
+ mlir::OpBuilder builder(mlirCtx.get());
+ builder.setInsertionPointToStart(&mlirModule->getRegion().front());
+ // Create a fake op to trip conversion to LLVM.
+ builder.create<test::dummy::FakeOp>(loc);
+
+ llvmCtx = std::make_unique<llvm::LLVMContext>();
+ }
+};
+
+TEST(CodeGenAction, GracefullyHandleLLVMConversionFailure) {
+ std::string diagnosticOutput;
+ llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
+ auto diagPrinter = std::make_unique<Fortran::frontend::TextDiagnosticPrinter>(
+ diagnosticsOS, new clang::DiagnosticOptions());
+
+ CompilerInstance ci;
+ ci.createDiagnostics(diagPrinter.get(), /*ShouldOwnClient=*/false);
+ ci.setInvocation(std::make_shared<CompilerInvocation>());
+ ci.setOutputStream(std::make_unique<llvm::raw_null_ostream>());
+ ci.getInvocation().getCodeGenOpts().OptimizationLevel = 0;
+
+ FrontendInputFile file("/dev/null", InputKind());
+
+ LLVMConversionFailureCodeGenAction action;
+ action.setInstance(&ci);
+ action.setCurrentInput(file);
+
+ consumeError(action.execute());
+ ASSERT_EQ(diagnosticsOS.str(),
+ "error: Lowering to LLVM IR failed\n"
+ "error: failed to create the LLVM module\n");
+}
>From 1661f886f50f82a0245d33de1924a76ca28daeab Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Tue, 16 Jan 2024 07:40:12 -0600
Subject: [PATCH 2/2] [MLIR][OpenMP] Better error reporting for unsupported
`nowait`
Provides some context for failing to generate LLVM IR for `target
enter|exit|update` directives when `nowait` is provided.
---
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 9 +++--
.../Target/LLVMIR/omptarget-nowait-llvm.mlir | 39 +++++++++++++++++++
2 files changed, 45 insertions(+), 3 deletions(-)
create mode 100644 mlir/test/Target/LLVMIR/omptarget-nowait-llvm.mlir
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index e7aebc3ce4be56..f8088bbd5eaca1 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1884,7 +1884,8 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
})
.Case([&](omp::EnterDataOp enterDataOp) {
if (enterDataOp.getNowait())
- return failure();
+ return (LogicalResult)(enterDataOp.emitError(
+ "`nowait` is not supported yet"));
if (auto ifExprVar = enterDataOp.getIfExpr())
ifCond = moduleTranslation.lookupValue(ifExprVar);
@@ -1900,7 +1901,8 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
})
.Case([&](omp::ExitDataOp exitDataOp) {
if (exitDataOp.getNowait())
- return failure();
+ return (LogicalResult)(exitDataOp.emitError(
+ "`nowait` is not supported yet"));
if (auto ifExprVar = exitDataOp.getIfExpr())
ifCond = moduleTranslation.lookupValue(ifExprVar);
@@ -1917,7 +1919,8 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
})
.Case([&](omp::UpdateDataOp updateDataOp) {
if (updateDataOp.getNowait())
- return failure();
+ return (LogicalResult)(updateDataOp.emitError(
+ "`nowait` is not supported yet"));
if (auto ifExprVar = updateDataOp.getIfExpr())
ifCond = moduleTranslation.lookupValue(ifExprVar);
diff --git a/mlir/test/Target/LLVMIR/omptarget-nowait-llvm.mlir b/mlir/test/Target/LLVMIR/omptarget-nowait-llvm.mlir
new file mode 100644
index 00000000000000..50c2f0702e63e8
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-nowait-llvm.mlir
@@ -0,0 +1,39 @@
+// RUN: not mlir-translate -mlir-to-llvmir -split-input-file %s 2>&1 | FileCheck %s
+
+llvm.func @_QPopenmp_target_data_update() {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, operand_segment_sizes = array<i32: 0, 0>, uniq_name = "_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr
+ %2 = omp.map_info var_ptr(%1 : !llvm.ptr, i32) map_clauses(to) capture(ByRef) -> !llvm.ptr {name = ""}
+
+ // CHECK: error: `nowait` is not supported yet
+ omp.target_update_data motion_entries(%2 : !llvm.ptr) nowait
+
+ llvm.return
+}
+
+// -----
+
+llvm.func @_QPopenmp_target_data_enter() {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, operand_segment_sizes = array<i32: 0, 0>, uniq_name = "_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr
+ %2 = omp.map_info var_ptr(%1 : !llvm.ptr, i32) map_clauses(to) capture(ByRef) -> !llvm.ptr {name = ""}
+
+ // CHECK: error: `nowait` is not supported yet
+ omp.target_enter_data map_entries(%2 : !llvm.ptr) nowait
+
+ llvm.return
+}
+
+
+// -----
+
+llvm.func @_QPopenmp_target_data_exit() {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, operand_segment_sizes = array<i32: 0, 0>, uniq_name = "_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr
+ %2 = omp.map_info var_ptr(%1 : !llvm.ptr, i32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
+
+ // CHECK: error: `nowait` is not supported yet
+ omp.target_exit_data map_entries(%2 : !llvm.ptr) nowait
+
+ llvm.return
+}
More information about the Mlir-commits
mailing list