[Mlir-commits] [mlir] [mlir][OpenMP] Fix sanitizer error in buildTaskLikeBodyGenCallback (PR #174983)
Tom Eccles
llvmlistbot at llvm.org
Thu Jan 8 06:30:19 PST 2026
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/174983
>From ef734dde72b9fc3f8a122098f6da7b08e686df64 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 8 Jan 2026 12:16:59 +0000
Subject: [PATCH] [mlir][OpenMP] Fix sanitizer error in
buildTaskLikeBodyGenCallback
This is a fix for the asan bot after #174386
Failing bot: https://lab.llvm.org/buildbot/#/builders/24/builds/16371
This commit undoes a simplification I thought reduced copied+pasted
code. I will merge it like this now to unblock the bot, and then work
separately on a different way to share code between both callbacks.
---
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 273 +++++++++++-------
1 file changed, 172 insertions(+), 101 deletions(-)
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 7fef30206799b..3dbbdc6cd496d 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -2365,105 +2365,11 @@ void TaskContextStructManager::freeStructPtr() {
builder.CreateFree(structPtr);
}
-using TaskLikeBodyGenCallbackTy =
- std::function<llvm::Error(llvm::OpenMPIRBuilder::InsertPointTy allocaIP,
- llvm::OpenMPIRBuilder::InsertPointTy codegenIP)>;
-
-/// Build the body generation callback shared by task-like constructs (task and
-/// taskloop).
-static TaskLikeBodyGenCallbackTy buildTaskLikeBodyGenCallback(
- Operation *opInst, Region ®ion, StringRef regionName,
- llvm::IRBuilderBase &builder, LLVM::ModuleTranslation &moduleTranslation,
- PrivateVarsInfo &privateVarsInfo, TaskContextStructManager &taskStructMgr) {
- using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
- return [&, regionName](InsertPointTy allocaIP,
- InsertPointTy codegenIP) -> llvm::Error {
- // Save the alloca insertion point on ModuleTranslation stack for use in
- // nested regions.
- LLVM::ModuleTranslation::SaveStack<OpenMPAllocaStackFrame> frame(
- moduleTranslation, allocaIP);
-
- // translate the body of the task:
- builder.restoreIP(codegenIP);
-
- llvm::BasicBlock *privInitBlock = nullptr;
- privateVarsInfo.llvmVars.resize(privateVarsInfo.blockArgs.size());
- for (auto [i, zip] : llvm::enumerate(llvm::zip_equal(
- privateVarsInfo.blockArgs, privateVarsInfo.privatizers,
- privateVarsInfo.mlirVars))) {
- auto [blockArg, privDecl, mlirPrivVar] = zip;
- // This is handled before the task executes
- if (privDecl.readsFromMold())
- continue;
-
- llvm::IRBuilderBase::InsertPointGuard guard(builder);
- llvm::Type *llvmAllocType =
- moduleTranslation.convertType(privDecl.getType());
- builder.SetInsertPoint(allocaIP.getBlock()->getTerminator());
- llvm::Value *llvmPrivateVar = builder.CreateAlloca(
- llvmAllocType, /*ArraySize=*/nullptr, "omp.private.alloc");
-
- llvm::Expected<llvm::Value *> privateVarOrError =
- initPrivateVar(builder, moduleTranslation, privDecl, mlirPrivVar,
- blockArg, llvmPrivateVar, privInitBlock);
- if (!privateVarOrError)
- return privateVarOrError.takeError();
- moduleTranslation.mapValue(blockArg, privateVarOrError.get());
- privateVarsInfo.llvmVars[i] = privateVarOrError.get();
- }
-
- taskStructMgr.createGEPsToPrivateVars();
- for (auto [i, llvmPrivVar] :
- llvm::enumerate(taskStructMgr.getLLVMPrivateVarGEPs())) {
- if (!llvmPrivVar) {
- assert(privateVarsInfo.llvmVars[i] &&
- "This is added in the loop above");
- continue;
- }
- privateVarsInfo.llvmVars[i] = llvmPrivVar;
- }
-
- // Find and map the addresses of each variable within the task context
- // structure
- for (auto [blockArg, llvmPrivateVar, privateDecl] :
- llvm::zip_equal(privateVarsInfo.blockArgs, privateVarsInfo.llvmVars,
- privateVarsInfo.privatizers)) {
- // This was handled above.
- if (!privateDecl.readsFromMold())
- continue;
- // Fix broken pass-by-value case for Fortran character boxes
- if (!mlir::isa<LLVM::LLVMPointerType>(blockArg.getType())) {
- llvmPrivateVar = builder.CreateLoad(
- moduleTranslation.convertType(blockArg.getType()), llvmPrivateVar);
- }
- assert(llvmPrivateVar->getType() ==
- moduleTranslation.convertType(blockArg.getType()));
- moduleTranslation.mapValue(blockArg, llvmPrivateVar);
- }
-
- auto continuationBlockOrError =
- convertOmpOpRegions(region, regionName, builder, moduleTranslation);
- if (failed(handleError(continuationBlockOrError, *opInst)))
- return llvm::make_error<PreviouslyReportedError>();
-
- builder.SetInsertPoint(continuationBlockOrError.get()->getTerminator());
-
- if (failed(cleanupPrivateVars(builder, moduleTranslation, opInst->getLoc(),
- privateVarsInfo.llvmVars,
- privateVarsInfo.privatizers)))
- return llvm::make_error<PreviouslyReportedError>();
-
- // Free heap allocated task context structure at the end of the task.
- taskStructMgr.freeStructPtr();
-
- return llvm::Error::success();
- };
-}
-
/// Converts an OpenMP task construct into LLVM IR using OpenMPIRBuilder.
static LogicalResult
convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
LLVM::ModuleTranslation &moduleTranslation) {
+ using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
if (failed(checkImplementationStatus(*taskOp)))
return failure();
@@ -2576,9 +2482,88 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
// Set up for call to createTask()
builder.SetInsertPoint(taskStartBlock);
- auto bodyCB = buildTaskLikeBodyGenCallback(
- taskOp, taskOp.getRegion(), "omp.task.region", builder, moduleTranslation,
- privateVarsInfo, taskStructMgr);
+ auto bodyCB = [&](InsertPointTy allocaIP,
+ InsertPointTy codegenIP) -> llvm::Error {
+ // Save the alloca insertion point on ModuleTranslation stack for use in
+ // nested regions.
+ LLVM::ModuleTranslation::SaveStack<OpenMPAllocaStackFrame> frame(
+ moduleTranslation, allocaIP);
+
+ // translate the body of the task:
+ builder.restoreIP(codegenIP);
+
+ llvm::BasicBlock *privInitBlock = nullptr;
+ privateVarsInfo.llvmVars.resize(privateVarsInfo.blockArgs.size());
+ for (auto [i, zip] : llvm::enumerate(llvm::zip_equal(
+ privateVarsInfo.blockArgs, privateVarsInfo.privatizers,
+ privateVarsInfo.mlirVars))) {
+ auto [blockArg, privDecl, mlirPrivVar] = zip;
+ // This is handled before the task executes
+ if (privDecl.readsFromMold())
+ continue;
+
+ llvm::IRBuilderBase::InsertPointGuard guard(builder);
+ llvm::Type *llvmAllocType =
+ moduleTranslation.convertType(privDecl.getType());
+ builder.SetInsertPoint(allocaIP.getBlock()->getTerminator());
+ llvm::Value *llvmPrivateVar = builder.CreateAlloca(
+ llvmAllocType, /*ArraySize=*/nullptr, "omp.private.alloc");
+
+ llvm::Expected<llvm::Value *> privateVarOrError =
+ initPrivateVar(builder, moduleTranslation, privDecl, mlirPrivVar,
+ blockArg, llvmPrivateVar, privInitBlock);
+ if (!privateVarOrError)
+ return privateVarOrError.takeError();
+ moduleTranslation.mapValue(blockArg, privateVarOrError.get());
+ privateVarsInfo.llvmVars[i] = privateVarOrError.get();
+ }
+
+ taskStructMgr.createGEPsToPrivateVars();
+ for (auto [i, llvmPrivVar] :
+ llvm::enumerate(taskStructMgr.getLLVMPrivateVarGEPs())) {
+ if (!llvmPrivVar) {
+ assert(privateVarsInfo.llvmVars[i] &&
+ "This is added in the loop above");
+ continue;
+ }
+ privateVarsInfo.llvmVars[i] = llvmPrivVar;
+ }
+
+ // Find and map the addresses of each variable within the task context
+ // structure
+ for (auto [blockArg, llvmPrivateVar, privateDecl] :
+ llvm::zip_equal(privateVarsInfo.blockArgs, privateVarsInfo.llvmVars,
+ privateVarsInfo.privatizers)) {
+ // This was handled above.
+ if (!privateDecl.readsFromMold())
+ continue;
+ // Fix broken pass-by-value case for Fortran character boxes
+ if (!mlir::isa<LLVM::LLVMPointerType>(blockArg.getType())) {
+ llvmPrivateVar = builder.CreateLoad(
+ moduleTranslation.convertType(blockArg.getType()), llvmPrivateVar);
+ }
+ assert(llvmPrivateVar->getType() ==
+ moduleTranslation.convertType(blockArg.getType()));
+ moduleTranslation.mapValue(blockArg, llvmPrivateVar);
+ }
+
+ auto continuationBlockOrError = convertOmpOpRegions(
+ taskOp.getRegion(), "omp.task.region", builder, moduleTranslation);
+ if (failed(handleError(continuationBlockOrError, *taskOp)))
+ return llvm::make_error<PreviouslyReportedError>();
+
+ builder.SetInsertPoint(continuationBlockOrError.get()->getTerminator());
+
+ if (failed(cleanupPrivateVars(builder, moduleTranslation, taskOp.getLoc(),
+ privateVarsInfo.llvmVars,
+ privateVarsInfo.privatizers)))
+ return llvm::make_error<PreviouslyReportedError>();
+
+ // Free heap allocated task context structure at the end of the task.
+ taskStructMgr.freeStructPtr();
+
+ return llvm::Error::success();
+ };
llvm::OpenMPIRBuilder &ompBuilder = *moduleTranslation.getOpenMPBuilder();
SmallVector<llvm::BranchInst *> cancelTerminators;
@@ -2699,9 +2684,95 @@ convertOmpTaskloopOp(Operation &opInst, llvm::IRBuilderBase &builder,
// Set up inserttion point for call to createTaskloop()
builder.SetInsertPoint(taskloopStartBlock);
- auto bodyCB = buildTaskLikeBodyGenCallback(
- &opInst, taskloopOp.getRegion(), "omp.taskloop.region", builder,
- moduleTranslation, privateVarsInfo, taskStructMgr);
+ auto bodyCB = [&](InsertPointTy allocaIP,
+ InsertPointTy codegenIP) -> llvm::Error {
+ // Save the alloca insertion point on ModuleTranslation stack for use in
+ // nested regions.
+ LLVM::ModuleTranslation::SaveStack<OpenMPAllocaStackFrame> frame(
+ moduleTranslation, allocaIP);
+
+ // translate the body of the taskloop:
+ builder.restoreIP(codegenIP);
+
+ llvm::BasicBlock *privInitBlock = nullptr;
+ privateVarsInfo.llvmVars.resize(privateVarsInfo.blockArgs.size());
+ for (auto [i, zip] : llvm::enumerate(llvm::zip_equal(
+ privateVarsInfo.blockArgs, privateVarsInfo.privatizers,
+ privateVarsInfo.mlirVars))) {
+ auto [blockArg, privDecl, mlirPrivVar] = zip;
+ // This is handled before the task executes
+ if (privDecl.readsFromMold())
+ continue;
+
+ llvm::IRBuilderBase::InsertPointGuard guard(builder);
+ llvm::Type *llvmAllocType =
+ moduleTranslation.convertType(privDecl.getType());
+ builder.SetInsertPoint(allocaIP.getBlock()->getTerminator());
+ llvm::Value *llvmPrivateVar = builder.CreateAlloca(
+ llvmAllocType, /*ArraySize=*/nullptr, "omp.private.alloc");
+
+ llvm::Expected<llvm::Value *> privateVarOrError =
+ initPrivateVar(builder, moduleTranslation, privDecl, mlirPrivVar,
+ blockArg, llvmPrivateVar, privInitBlock);
+ if (!privateVarOrError)
+ return privateVarOrError.takeError();
+ moduleTranslation.mapValue(blockArg, privateVarOrError.get());
+ privateVarsInfo.llvmVars[i] = privateVarOrError.get();
+ }
+
+ taskStructMgr.createGEPsToPrivateVars();
+ for (auto [i, llvmPrivVar] :
+ llvm::enumerate(taskStructMgr.getLLVMPrivateVarGEPs())) {
+ if (!llvmPrivVar) {
+ assert(privateVarsInfo.llvmVars[i] &&
+ "This is added in the loop above");
+ continue;
+ }
+ privateVarsInfo.llvmVars[i] = llvmPrivVar;
+ }
+
+ // Find and map the addresses of each variable within the taskloop context
+ // structure
+ for (auto [blockArg, llvmPrivateVar, privateDecl] :
+ llvm::zip_equal(privateVarsInfo.blockArgs, privateVarsInfo.llvmVars,
+ privateVarsInfo.privatizers)) {
+ // This was handled above.
+ if (!privateDecl.readsFromMold())
+ continue;
+ // Fix broken pass-by-value case for Fortran character boxes
+ if (!mlir::isa<LLVM::LLVMPointerType>(blockArg.getType())) {
+ llvmPrivateVar = builder.CreateLoad(
+ moduleTranslation.convertType(blockArg.getType()), llvmPrivateVar);
+ }
+ assert(llvmPrivateVar->getType() ==
+ moduleTranslation.convertType(blockArg.getType()));
+ moduleTranslation.mapValue(blockArg, llvmPrivateVar);
+ }
+
+ auto continuationBlockOrError =
+ convertOmpOpRegions(taskloopOp.getRegion(), "omp.taskloop.region",
+ builder, moduleTranslation);
+
+ if (failed(handleError(continuationBlockOrError, opInst)))
+ return llvm::make_error<PreviouslyReportedError>();
+
+ builder.SetInsertPoint(continuationBlockOrError.get()->getTerminator());
+
+ // This is freeing the private variables as mapped inside of the task: these
+ // will be per-task private copies possibly after task duplication. This is
+ // handled transparently by how these are passed to the structure passed
+ // into the outlined function. When the task is duplicated, that structure
+ // is duplicated too.
+ if (failed(cleanupPrivateVars(builder, moduleTranslation,
+ taskloopOp.getLoc(), privateVarsInfo.llvmVars,
+ privateVarsInfo.privatizers)))
+ return llvm::make_error<PreviouslyReportedError>();
+ // Similarly, the task context structure freed inside the task is the
+ // per-task copy after task duplication.
+ taskStructMgr.freeStructPtr();
+
+ return llvm::Error::success();
+ };
// Taskloop divides into an appropriate number of tasks by repeatedly
// duplicating the original task. Each time this is done, the task context
More information about the Mlir-commits
mailing list