[Mlir-commits] [llvm] [mlir] [MLIR][OpenMP][OMPIRBuilder] Error propagation across callbacks (PR #112533)
Sergio Afonso
llvmlistbot at llvm.org
Wed Oct 16 05:47:03 PDT 2024
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/112533
>From 201a1fa960b093c7ee8a689ae1e363beec49391f Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Wed, 16 Oct 2024 13:18:51 +0100
Subject: [PATCH] [MLIR][OpenMP][OMPIRBuilder] Error propagation across
callbacks
This is a small proof of concept showing an approach to communicate errors
between MLIR to LLVM IR translation of the OpenMP dialect and the OMPIRBuilder.
It only implements the approach for a single case, so it doesn't compile or
run, since it's only intended to show how it could look like and discuss it
before investing too much effort on a full implementation.
The main idea is to use `llvm::Error` objects returned by callbacks passed to
`OMPIRBuilder` codegen functions that they can then check and forward back to
the caller to avoid continuing after an error has been hit. The caller then
emits an MLIR error diagnostic based on that and stops the translation process.
This should prevent encountering any unsupported operations or arguments, or
any other unexpected error from resulting in a compiler crash. Instead, a
descriptive error message is presented to users.
---
.../llvm/Frontend/OpenMP/OMPIRBuilder.h | 24 ++++++++------
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 5 +--
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 32 +++++++++++--------
3 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 8834c3b1f50115..e75fdeed0d2ed7 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -589,15 +589,19 @@ class OpenMPIRBuilder {
/// not be split.
/// \param CodeGenIP is the insertion point at which the body code should be
/// placed.
+ ///
+ /// \return an error, if any were triggered during execution.
using BodyGenCallbackTy =
- function_ref<void(InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
+ function_ref<Error(InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
// This is created primarily for sections construct as llvm::function_ref
// (BodyGenCallbackTy) is not storable (as described in the comments of
// function_ref class - function_ref contains non-ownable reference
// to the callable.
+ ///
+ /// \return an error, if any were triggered during execution.
using StorableBodyGenCallbackTy =
- std::function<void(InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
+ std::function<Error(InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
/// Callback type for loop body code generation.
///
@@ -607,8 +611,10 @@ class OpenMPIRBuilder {
/// terminated with an unconditional branch to the loop
/// latch.
/// \param IndVar is the induction variable usable at the insertion point.
+ ///
+ /// \return an error, if any were triggered during execution.
using LoopBodyGenCallbackTy =
- function_ref<void(InsertPointTy CodeGenIP, Value *IndVar)>;
+ function_ref<Error(InsertPointTy CodeGenIP, Value *IndVar)>;
/// Callback type for variable privatization (think copy & default
/// constructor).
@@ -626,9 +632,9 @@ class OpenMPIRBuilder {
/// \param ReplVal The replacement value, thus a copy or new created version
/// of \p Inner.
///
- /// \returns The new insertion point where code generation continues and
- /// \p ReplVal the replacement value.
- using PrivatizeCallbackTy = function_ref<InsertPointTy(
+ /// \returns The new insertion point where code generation continues or an
+ /// error, and \p ReplVal the replacement value.
+ using PrivatizeCallbackTy = function_ref<Expected<InsertPointTy>(
InsertPointTy AllocaIP, InsertPointTy CodeGenIP, Value &Original,
Value &Inner, Value *&ReplVal)>;
@@ -1262,9 +1268,9 @@ class OpenMPIRBuilder {
/// \param Loc The location where the taskgroup construct was encountered.
/// \param AllocaIP The insertion point to be used for alloca instructions.
/// \param BodyGenCB Callback that will generate the region code.
- InsertPointTy createTaskgroup(const LocationDescription &Loc,
- InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB);
+ Expected<InsertPointTy> createTaskgroup(const LocationDescription &Loc,
+ InsertPointTy AllocaIP,
+ BodyGenCallbackTy BodyGenCB);
using FileIdentifierInfoCallbackTy =
std::function<std::tuple<std::string, uint64_t>()>;
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 172812a3802d33..6d040c349a6d65 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2048,7 +2048,7 @@ OpenMPIRBuilder::createTask(const LocationDescription &Loc,
return Builder.saveIP();
}
-OpenMPIRBuilder::InsertPointTy
+Expected<OpenMPIRBuilder::InsertPointTy>
OpenMPIRBuilder::createTaskgroup(const LocationDescription &Loc,
InsertPointTy AllocaIP,
BodyGenCallbackTy BodyGenCB) {
@@ -2066,7 +2066,8 @@ OpenMPIRBuilder::createTaskgroup(const LocationDescription &Loc,
Builder.CreateCall(TaskgroupFn, {Ident, ThreadID});
BasicBlock *TaskgroupExitBB = splitBB(Builder, true, "taskgroup.exit");
- BodyGenCB(AllocaIP, Builder.saveIP());
+ if (auto Err = BodyGenCB(AllocaIP, Builder.saveIP()))
+ return std::move(Err);
Builder.SetInsertPoint(TaskgroupExitBB);
// Emit the @__kmpc_end_taskgroup runtime call to end the taskgroup
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index cb7dd3cd874d9f..9bf65fb79a5682 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -137,9 +137,9 @@ findAllocaInsertPoint(llvm::IRBuilderBase &builder,
/// region, and a branch from any block with an successor-less OpenMP terminator
/// to `continuationBlock`. Populates `continuationBlockPHIs` with the PHI nodes
/// of the continuation block if provided.
-static llvm::BasicBlock *convertOmpOpRegions(
+static llvm::Expected<llvm::BasicBlock *> convertOmpOpRegions(
Region ®ion, StringRef blockName, llvm::IRBuilderBase &builder,
- LLVM::ModuleTranslation &moduleTranslation, LogicalResult &bodyGenStatus,
+ LLVM::ModuleTranslation &moduleTranslation,
SmallVectorImpl<llvm::PHINode *> *continuationBlockPHIs = nullptr) {
llvm::BasicBlock *continuationBlock =
splitBB(builder, true, "omp.region.cont");
@@ -215,10 +215,8 @@ static llvm::BasicBlock *convertOmpOpRegions(
llvm::IRBuilderBase::InsertPointGuard guard(builder);
if (failed(
- moduleTranslation.convertBlock(*bb, bb->isEntryBlock(), builder))) {
- bodyGenStatus = failure();
- return continuationBlock;
- }
+ moduleTranslation.convertBlock(*bb, bb->isEntryBlock(), builder)))
+ return llvm::createStringError("failed region translation");
// Special handling for `omp.yield` and `omp.terminator` (we may have more
// than one): they return the control to the parent OpenMP dialect operation
@@ -1145,20 +1143,26 @@ static LogicalResult
convertOmpTaskgroupOp(omp::TaskgroupOp tgOp, llvm::IRBuilderBase &builder,
LLVM::ModuleTranslation &moduleTranslation) {
using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
- LogicalResult bodyGenStatus = success();
- if (!tgOp.getTaskReductionVars().empty() || !tgOp.getAllocateVars().empty()) {
+ if (!tgOp.getTaskReductionVars().empty() || !tgOp.getAllocateVars().empty())
return tgOp.emitError("unhandled clauses for translation to LLVM IR");
- }
+
auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
builder.restoreIP(codegenIP);
- convertOmpOpRegions(tgOp.getRegion(), "omp.taskgroup.region", builder,
- moduleTranslation, bodyGenStatus);
+ return convertOmpOpRegions(tgOp.getRegion(), "omp.taskgroup.region",
+ builder, moduleTranslation)
+ .takeError();
};
+
InsertPointTy allocaIP = findAllocaInsertPoint(builder, moduleTranslation);
llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
- builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createTaskgroup(
- ompLoc, allocaIP, bodyCB));
- return bodyGenStatus;
+ auto result = moduleTranslation.getOpenMPBuilder()->createTaskgroup(
+ ompLoc, allocaIP, bodyCB);
+
+ if (!result)
+ return tgOp.emitError(llvm::toString(result.takeError()));
+
+ builder.restoreIP(*result);
+ return success();
}
static LogicalResult
More information about the Mlir-commits
mailing list