[MLIR][OpenMP] Better error reporting for unsupported nowait
#78551
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
mlir:llvm
flang:driver
mlir
flang
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-mlir-llvm Author: Kareem Ergawy (ergawy) ChangesIf This is a follow-up to #78269, please only review the latest commit. Full diff: https://github.com/llvm/llvm-project/pull/78551.diff 5 Files Affected:
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");
+}
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
+}
|
|
@llvm/pr-subscribers-mlir Author: Kareem Ergawy (ergawy) ChangesIf This is a follow-up to #78269, please only review the latest commit. Full diff: https://github.com/llvm/llvm-project/pull/78551.diff 5 Files Affected:
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");
+}
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
+}
|
ergawy
changed the title
CodeGenAction::executeAction()nowait
Provides some context for failing to generate LLVM IR for `target enter|exit|update` directives when `nowait` is provided.
ergawy
force-pushed
the
fix_segfault_flang_2
branch
from
1661f88 to
fd2bfe2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Provides some context for failing to generate LLVM IR for
target enter|exit|updatedirectives whennowaitis provided. This is directly helpful for flang users since they would get this error message if they tried to usenowait. Before that we had a very generic message.This is a follow-up to #78269, please only review the latest commit (the one with the same commit message as the PR title).