[OMPIRBuilder] Add support for explicit deallocation points #154752
+629
−514
Conversation
This file contains hidden or 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
skatrak
requested review from
Meinersbur,
tblah,
NimishMishra,
TIFitis and
bhandarkar-pranav
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Sergio Afonso (skatrak) ChangesIn this patch, some OMPIRBuilder codegen functions and callbacks are updated to work with arrays of deallocation insertion points. The purpose of this is to enable the replacement of The OpenMP to LLVM IR MLIR translation pass is updated to properly store and forward deallocation points together with their matching allocation point to the OMPIRBuilder. Currently, only the Instead of a single deallocation point, lists of those are used. This is to cover cases where there are multiple exit blocks originating from a single entry. If an allocation needing explicit deallocation is placed in the entry block of such cases, it would need to be deallocated before each of the exits. Patch is 143.00 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/154752.diff 15 Files Affected:
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index f98339d472fa9..f0cb7531845c8 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -10500,8 +10500,8 @@ void CGOpenMPRuntime::emitTargetDataCalls(
llvm::OpenMPIRBuilder::LocationDescription OmpLoc(CodeGenIP);
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
cantFail(OMPBuilder.createTargetData(
- OmpLoc, AllocaIP, CodeGenIP, DeviceID, IfCondVal, Info, GenMapInfoCB,
- CustomMapperCB,
+ OmpLoc, AllocaIP, CodeGenIP, /*DeallocIPs=*/{}, DeviceID, IfCondVal,
+ Info, GenMapInfoCB, CustomMapperCB,
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, RTLoc));
CGF.Builder.restoreIP(AfterIP);
}
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index f6a0ca574a191..959c66593b157 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1835,10 +1835,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
const CapturedStmt *CS = S.getCapturedStmt(OMPD_parallel);
const Stmt *ParallelRegionBodyStmt = CS->getCapturedStmt();
- auto BodyGenCB = [&, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [&, this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPOutlinedRegionBody(
- *this, ParallelRegionBodyStmt, AllocaIP, CodeGenIP, "parallel");
+ *this, ParallelRegionBodyStmt, AllocIP, CodeGenIP, "parallel");
return llvm::Error::success();
};
@@ -1846,9 +1846,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
- llvm::OpenMPIRBuilder::InsertPointTy AfterIP = cantFail(
- OMPBuilder.createParallel(Builder, AllocaIP, BodyGenCB, PrivCB, FiniCB,
- IfCond, NumThreads, ProcBind, S.hasCancel()));
+ llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
+ cantFail(OMPBuilder.createParallel(
+ Builder, AllocaIP, /*DeallocIPs=*/{}, BodyGenCB, PrivCB, FiniCB,
+ IfCond, NumThreads, ProcBind, S.hasCancel()));
Builder.restoreIP(AfterIP);
return;
}
@@ -4361,19 +4362,21 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
llvm::SmallVector<BodyGenCallbackTy, 4> SectionCBVector;
if (CS) {
for (const Stmt *SubStmt : CS->children()) {
- auto SectionCB = [this, SubStmt](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto SectionCB = [this, SubStmt](InsertPointTy AllocIP,
+ InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, SubStmt, AllocaIP, CodeGenIP, "section");
+ *this, SubStmt, AllocIP, CodeGenIP, "section");
return llvm::Error::success();
};
SectionCBVector.push_back(SectionCB);
}
} else {
- auto SectionCB = [this, CapturedStmt](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto SectionCB = [this, CapturedStmt](
+ InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, CapturedStmt, AllocaIP, CodeGenIP, "section");
+ *this, CapturedStmt, AllocIP, CodeGenIP, "section");
return llvm::Error::success();
};
SectionCBVector.push_back(SectionCB);
@@ -4429,10 +4432,11 @@ void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [SectionRegionBodyStmt, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [SectionRegionBodyStmt,
+ this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, SectionRegionBodyStmt, AllocaIP, CodeGenIP, "section");
+ *this, SectionRegionBodyStmt, AllocIP, CodeGenIP, "section");
return llvm::Error::success();
};
@@ -4514,10 +4518,11 @@ void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [MasterRegionBodyStmt,
+ this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, MasterRegionBodyStmt, AllocaIP, CodeGenIP, "master");
+ *this, MasterRegionBodyStmt, AllocIP, CodeGenIP, "master");
return llvm::Error::success();
};
@@ -4564,10 +4569,11 @@ void CodeGenFunction::EmitOMPMaskedDirective(const OMPMaskedDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [MaskedRegionBodyStmt, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [MaskedRegionBodyStmt,
+ this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, MaskedRegionBodyStmt, AllocaIP, CodeGenIP, "masked");
+ *this, MaskedRegionBodyStmt, AllocIP, CodeGenIP, "masked");
return llvm::Error::success();
};
@@ -4607,10 +4613,11 @@ void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [CriticalRegionBodyStmt, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [CriticalRegionBodyStmt,
+ this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, CriticalRegionBodyStmt, AllocaIP, CodeGenIP, "critical");
+ *this, CriticalRegionBodyStmt, AllocIP, CodeGenIP, "critical");
return llvm::Error::success();
};
@@ -5577,8 +5584,8 @@ void CodeGenFunction::EmitOMPTaskgroupDirective(
InsertPointTy AllocaIP(AllocaInsertPt->getParent(),
AllocaInsertPt->getIterator());
- auto BodyGenCB = [&, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [&, this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
Builder.restoreIP(CodeGenIP);
EmitStmt(S.getInnermostCapturedStmt()->getCapturedStmt());
return llvm::Error::success();
@@ -5587,7 +5594,8 @@ void CodeGenFunction::EmitOMPTaskgroupDirective(
if (!CapturedStmtInfo)
CapturedStmtInfo = &CapStmtInfo;
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
- cantFail(OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB));
+ cantFail(OMPBuilder.createTaskgroup(Builder, AllocaIP,
+ /*DeallocIPs=*/{}, BodyGenCB));
Builder.restoreIP(AfterIP);
return;
}
@@ -6167,8 +6175,9 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [&S, C, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [&S, C, this](InsertPointTy AllocIP,
+ InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
Builder.restoreIP(CodeGenIP);
const CapturedStmt *CS = S.getInnermostCapturedStmt();
@@ -6186,7 +6195,7 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
OutlinedFn, CapturedVars);
} else {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, CS->getCapturedStmt(), AllocaIP, CodeGenIP, "ordered");
+ *this, CS->getCapturedStmt(), AllocIP, CodeGenIP, "ordered");
}
return llvm::Error::success();
};
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 36be9bfd999a1..d033691a8db45 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -602,17 +602,19 @@ class OpenMPIRBuilder {
/// such InsertPoints need to be preserved, it can split the block itself
/// before calling the callback.
///
- /// AllocaIP and CodeGenIP must not point to the same position.
- ///
- /// \param AllocaIP is the insertion point at which new alloca instructions
- /// should be placed. The BasicBlock it is pointing to must
- /// not be split.
- /// \param CodeGenIP is the insertion point at which the body code should be
- /// placed.
- ///
+ /// AllocIP and CodeGenIP must not point to the same position.
+ ///
+ /// \param AllocIP is the insertion point at which new allocations should
+ /// be placed. The BasicBlock it is pointing to must not be
+ /// split.
+ /// \param CodeGenIP is the insertion point at which the body code should be
+ /// placed.
+ /// \param DeallocIPs is the list of insertion points where explicit
+ /// deallocations, if needed, should be placed.
/// \return an error, if any were triggered during execution.
using BodyGenCallbackTy =
- function_ref<Error(InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
+ function_ref<Error(InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs)>;
// This is created primarily for sections construct as llvm::function_ref
// (BodyGenCallbackTy) is not storable (as described in the comments of
@@ -621,7 +623,8 @@ class OpenMPIRBuilder {
///
/// \return an error, if any were triggered during execution.
using StorableBodyGenCallbackTy =
- std::function<Error(InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
+ std::function<Error(InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs)>;
/// Callback type for loop body code generation.
///
@@ -715,7 +718,9 @@ class OpenMPIRBuilder {
/// Generator for '#omp parallel'
///
/// \param Loc The insert and source location description.
- /// \param AllocaIP The insertion points to be used for alloca instructions.
+ /// \param AllocIP The insertion point to be used for allocations.
+ /// \param DeallocIPs The insertion points to be used for explicit
+ /// deallocations, if needed.
/// \param BodyGenCB Callback that will generate the region code.
/// \param PrivCB Callback to copy a given variable (think copy constructor).
/// \param FiniCB Callback to finalize variable copies.
@@ -726,10 +731,10 @@ class OpenMPIRBuilder {
///
/// \returns The insertion position *after* the parallel.
LLVM_ABI InsertPointOrErrorTy createParallel(
- const LocationDescription &Loc, InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB, PrivatizeCallbackTy PrivCB,
- FinalizeCallbackTy FiniCB, Value *IfCondition, Value *NumThreads,
- omp::ProcBindKind ProcBind, bool IsCancellable);
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ ArrayRef<InsertPointTy> DeallocIPs, BodyGenCallbackTy BodyGenCB,
+ PrivatizeCallbackTy PrivCB, FinalizeCallbackTy FiniCB, Value *IfCondition,
+ Value *NumThreads, omp::ProcBindKind ProcBind, bool IsCancellable);
/// Generator for the control flow structure of an OpenMP canonical loop.
///
@@ -1347,7 +1352,9 @@ class OpenMPIRBuilder {
/// Generator for `#omp task`
///
/// \param Loc The location where the task construct was encountered.
- /// \param AllocaIP The insertion point to be used for alloca instructions.
+ /// \param AllocIP The insertion point to be used for allocations.
+ /// \param DeallocIPs The insertion points to be used for explicit
+ /// deallocations, if needed.
/// \param BodyGenCB Callback that will generate the region code.
/// \param Tied True if the task is tied, false if the task is untied.
/// \param Final i1 value which is `true` if the task is final, `false` if the
@@ -1363,21 +1370,23 @@ class OpenMPIRBuilder {
/// \param Mergeable If the given task is `mergeable`
/// \param priority `priority-value' specifies the execution order of the
/// tasks that is generated by the construct
- LLVM_ABI InsertPointOrErrorTy
- createTask(const LocationDescription &Loc, InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB, bool Tied = true,
- Value *Final = nullptr, Value *IfCondition = nullptr,
- SmallVector<DependData> Dependencies = {}, bool Mergeable = false,
- Value *EventHandle = nullptr, Value *Priority = nullptr);
+ LLVM_ABI InsertPointOrErrorTy createTask(
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ ArrayRef<InsertPointTy> DeallocIPs, BodyGenCallbackTy BodyGenCB,
+ bool Tied = true, Value *Final = nullptr, Value *IfCondition = nullptr,
+ SmallVector<DependData> Dependencies = {}, bool Mergeable = false,
+ Value *EventHandle = nullptr, Value *Priority = nullptr);
/// Generator for the taskgroup construct
///
/// \param Loc The location where the taskgroup construct was encountered.
- /// \param AllocaIP The insertion point to be used for alloca instructions.
+ /// \param AllocIP The insertion point to be used for allocations.
+ /// \param DeallocIPs The insertion point to be used for explicit deallocation
+ /// instructions, if needed.
/// \param BodyGenCB Callback that will generate the region code.
- LLVM_ABI InsertPointOrErrorTy createTaskgroup(const LocationDescription &Loc,
- InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB);
+ LLVM_ABI InsertPointOrErrorTy createTaskgroup(
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ ArrayRef<InsertPointTy> DeallocIPs, BodyGenCallbackTy BodyGenCB);
using FileIdentifierInfoCallbackTy =
std::function<std::tuple<std::string, uint64_t>()>;
@@ -2246,7 +2255,8 @@ class OpenMPIRBuilder {
struct OutlineInfo {
using PostOutlineCBTy = std::function<void(Function &)>;
PostOutlineCBTy PostOutlineCB;
- BasicBlock *EntryBB, *ExitBB, *OuterAllocaBB;
+ BasicBlock *EntryBB, *ExitBB, *OuterAllocBB;
+ SmallVector<BasicBlock *> OuterDeallocBBs;
SmallVector<Value *, 2> ExcludeArgsFromAggregate;
LLVM_ABI virtual ~OutlineInfo() = default;
@@ -2319,7 +2329,8 @@ class OpenMPIRBuilder {
/// \return an error, if any were triggered during execution.
LLVM_ABI Error emitIfClause(Value *Cond, BodyGenCallbackTy ThenGen,
BodyGenCallbackTy ElseGen,
- InsertPointTy AllocaIP = {});
+ InsertPointTy AllocIP = {},
+ ArrayRef<InsertPointTy> DeallocIPs = {});
/// Create the global variable holding the offload mappings information.
LLVM_ABI GlobalVariable *
@@ -2874,11 +2885,13 @@ class OpenMPIRBuilder {
/// Generator for `#omp distribute`
///
/// \param Loc The location where the distribute construct was encountered.
- /// \param AllocaIP The insertion points to be used for alloca instructions.
+ /// \param AllocIP The insertion point to be used for allocations.
+ /// \param DeallocIPs The insertion points to be used for explicit
+ /// deallocations, if needed.
/// \param BodyGenCB Callback that will generate the region code.
- LLVM_ABI InsertPointOrErrorTy createDistribute(const LocationDescription &Loc,
- InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB);
+ LLVM_ABI InsertPointOrErrorTy createDistribute(
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ ArrayRef<InsertPointTy> DeallocIPs, BodyGenCallbackTy BodyGenCB);
/// Generate conditional branch and relevant BasicBlocks through which private
/// threads copy the 'copyin' variables from Master copy to threadprivate
@@ -3206,9 +3219,11 @@ class OpenMPIRBuilder {
/// Generator for '#omp target data'
///
/// \param Loc The location where the target data construct was encountered.
- /// \param AllocaIP The insertion points to be used for alloca instructions.
+ /// \param AllocIP The insertion points to be used for allocations.
/// \param CodeGenIP The insertion point at which the target directive code
/// should be placed.
+ /// \param DeallocIPs The insertion points at which explicit deallocations
+ /// should be placed, if needed.
/// \param IsBegin If true then emits begin mapper call otherwise emits
/// end mapper call.
/// \param DeviceID Stores the DeviceID from the device clause.
@@ -3221,10 +3236,10 @@ class OpenMPIRBuilder {
/// \param DeviceAddrCB Optional callback to generate code related to
/// use_device_ptr and use_device_addr.
LLVM_ABI InsertPointOrErrorTy createTargetData(
- const LocationDescription &Loc, InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP, Value *DeviceID, Value *IfCond,
- TargetDataInfo &Info, GenMapInfoCallbackTy GenMapInfoCB,
- CustomMapperCallbackTy CustomMapperCB,
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ InsertPointTy CodeGenIP, ArrayRef<InsertPointTy> DeallocIPs,
+ Value *DeviceID, Value *IfCond, TargetDataInfo &Info,
+ GenMapInfoCallbackTy GenMapInfoCB, CustomMapperCallbackTy CustomMapperCB,
omp::RuntimeFunction *MapperFunc = nullptr,
function_ref<InsertPointOrErrorTy(InsertPointTy CodeGenIP,
BodyGenTy BodyGenType)>
@@ -3233,7 +3248,8 @@ class OpenMPIRBuilder {
Value *SrcLocInfo = nullptr);
using TargetBodyGenCallbackTy = function_ref<InsertPointOrErrorTy(
- InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
+ InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs)>;
using TargetGenArgAccessorsCallbackTy = function_ref<InsertPointOrErrorTy(
Argument &Arg, Value *Input, Value *&RetVal, InsertPointTy AllocaIP,
@@ -3245,6 +3261,8 @@ class OpenMPIRBuilder {
/// \param IsOffloadEntry whether it is an offload entry.
/// \param CodeGenIP The insertion point where the call to the outlined
/// function should be emitted.
+ /// \param DeallocIPs The insertion points at which explicit deallocations
+ /// should be placed, if needed.
/// \param Info Stores all information realted to the Target directive.
/// \param EntryInfo The entry information about the function.
/// \param DefaultAttrs Structure containing the default attributes, including
@@ -3265,8 +3283,9 @@ class OpenMPIRBuilder {
/// not.
LLVM_ABI InsertPointOrErrorTy createTarget(
const LocationDescription &Loc, bool IsOffloadEntry,
- OpenMPIRBuilder::InsertPointTy AllocaIP,
- OpenMPIRBuilder::InsertPointTy CodeGenIP, TargetDataInfo &Info,
+ OpenMPIRBuilder::InsertPointTy AllocIP,
+ Open...
[truncated]
|
|
@llvm/pr-subscribers-mlir Author: Sergio Afonso (skatrak) ChangesIn this patch, some OMPIRBuilder codegen functions and callbacks are updated to work with arrays of deallocation insertion points. The purpose of this is to enable the replacement of The OpenMP to LLVM IR MLIR translation pass is updated to properly store and forward deallocation points together with their matching allocation point to the OMPIRBuilder. Currently, only the Instead of a single deallocation point, lists of those are used. This is to cover cases where there are multiple exit blocks originating from a single entry. If an allocation needing explicit deallocation is placed in the entry block of such cases, it would need to be deallocated before each of the exits. Patch is 143.00 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/154752.diff 15 Files Affected:
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index f98339d472fa9..f0cb7531845c8 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -10500,8 +10500,8 @@ void CGOpenMPRuntime::emitTargetDataCalls(
llvm::OpenMPIRBuilder::LocationDescription OmpLoc(CodeGenIP);
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
cantFail(OMPBuilder.createTargetData(
- OmpLoc, AllocaIP, CodeGenIP, DeviceID, IfCondVal, Info, GenMapInfoCB,
- CustomMapperCB,
+ OmpLoc, AllocaIP, CodeGenIP, /*DeallocIPs=*/{}, DeviceID, IfCondVal,
+ Info, GenMapInfoCB, CustomMapperCB,
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, RTLoc));
CGF.Builder.restoreIP(AfterIP);
}
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index f6a0ca574a191..959c66593b157 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1835,10 +1835,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
const CapturedStmt *CS = S.getCapturedStmt(OMPD_parallel);
const Stmt *ParallelRegionBodyStmt = CS->getCapturedStmt();
- auto BodyGenCB = [&, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [&, this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPOutlinedRegionBody(
- *this, ParallelRegionBodyStmt, AllocaIP, CodeGenIP, "parallel");
+ *this, ParallelRegionBodyStmt, AllocIP, CodeGenIP, "parallel");
return llvm::Error::success();
};
@@ -1846,9 +1846,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
- llvm::OpenMPIRBuilder::InsertPointTy AfterIP = cantFail(
- OMPBuilder.createParallel(Builder, AllocaIP, BodyGenCB, PrivCB, FiniCB,
- IfCond, NumThreads, ProcBind, S.hasCancel()));
+ llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
+ cantFail(OMPBuilder.createParallel(
+ Builder, AllocaIP, /*DeallocIPs=*/{}, BodyGenCB, PrivCB, FiniCB,
+ IfCond, NumThreads, ProcBind, S.hasCancel()));
Builder.restoreIP(AfterIP);
return;
}
@@ -4361,19 +4362,21 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
llvm::SmallVector<BodyGenCallbackTy, 4> SectionCBVector;
if (CS) {
for (const Stmt *SubStmt : CS->children()) {
- auto SectionCB = [this, SubStmt](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto SectionCB = [this, SubStmt](InsertPointTy AllocIP,
+ InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, SubStmt, AllocaIP, CodeGenIP, "section");
+ *this, SubStmt, AllocIP, CodeGenIP, "section");
return llvm::Error::success();
};
SectionCBVector.push_back(SectionCB);
}
} else {
- auto SectionCB = [this, CapturedStmt](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto SectionCB = [this, CapturedStmt](
+ InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, CapturedStmt, AllocaIP, CodeGenIP, "section");
+ *this, CapturedStmt, AllocIP, CodeGenIP, "section");
return llvm::Error::success();
};
SectionCBVector.push_back(SectionCB);
@@ -4429,10 +4432,11 @@ void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [SectionRegionBodyStmt, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [SectionRegionBodyStmt,
+ this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, SectionRegionBodyStmt, AllocaIP, CodeGenIP, "section");
+ *this, SectionRegionBodyStmt, AllocIP, CodeGenIP, "section");
return llvm::Error::success();
};
@@ -4514,10 +4518,11 @@ void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [MasterRegionBodyStmt,
+ this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, MasterRegionBodyStmt, AllocaIP, CodeGenIP, "master");
+ *this, MasterRegionBodyStmt, AllocIP, CodeGenIP, "master");
return llvm::Error::success();
};
@@ -4564,10 +4569,11 @@ void CodeGenFunction::EmitOMPMaskedDirective(const OMPMaskedDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [MaskedRegionBodyStmt, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [MaskedRegionBodyStmt,
+ this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, MaskedRegionBodyStmt, AllocaIP, CodeGenIP, "masked");
+ *this, MaskedRegionBodyStmt, AllocIP, CodeGenIP, "masked");
return llvm::Error::success();
};
@@ -4607,10 +4613,11 @@ void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [CriticalRegionBodyStmt, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [CriticalRegionBodyStmt,
+ this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, CriticalRegionBodyStmt, AllocaIP, CodeGenIP, "critical");
+ *this, CriticalRegionBodyStmt, AllocIP, CodeGenIP, "critical");
return llvm::Error::success();
};
@@ -5577,8 +5584,8 @@ void CodeGenFunction::EmitOMPTaskgroupDirective(
InsertPointTy AllocaIP(AllocaInsertPt->getParent(),
AllocaInsertPt->getIterator());
- auto BodyGenCB = [&, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [&, this](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
Builder.restoreIP(CodeGenIP);
EmitStmt(S.getInnermostCapturedStmt()->getCapturedStmt());
return llvm::Error::success();
@@ -5587,7 +5594,8 @@ void CodeGenFunction::EmitOMPTaskgroupDirective(
if (!CapturedStmtInfo)
CapturedStmtInfo = &CapStmtInfo;
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
- cantFail(OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB));
+ cantFail(OMPBuilder.createTaskgroup(Builder, AllocaIP,
+ /*DeallocIPs=*/{}, BodyGenCB));
Builder.restoreIP(AfterIP);
return;
}
@@ -6167,8 +6175,9 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
return llvm::Error::success();
};
- auto BodyGenCB = [&S, C, this](InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP) {
+ auto BodyGenCB = [&S, C, this](InsertPointTy AllocIP,
+ InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs) {
Builder.restoreIP(CodeGenIP);
const CapturedStmt *CS = S.getInnermostCapturedStmt();
@@ -6186,7 +6195,7 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
OutlinedFn, CapturedVars);
} else {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
- *this, CS->getCapturedStmt(), AllocaIP, CodeGenIP, "ordered");
+ *this, CS->getCapturedStmt(), AllocIP, CodeGenIP, "ordered");
}
return llvm::Error::success();
};
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 36be9bfd999a1..d033691a8db45 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -602,17 +602,19 @@ class OpenMPIRBuilder {
/// such InsertPoints need to be preserved, it can split the block itself
/// before calling the callback.
///
- /// AllocaIP and CodeGenIP must not point to the same position.
- ///
- /// \param AllocaIP is the insertion point at which new alloca instructions
- /// should be placed. The BasicBlock it is pointing to must
- /// not be split.
- /// \param CodeGenIP is the insertion point at which the body code should be
- /// placed.
- ///
+ /// AllocIP and CodeGenIP must not point to the same position.
+ ///
+ /// \param AllocIP is the insertion point at which new allocations should
+ /// be placed. The BasicBlock it is pointing to must not be
+ /// split.
+ /// \param CodeGenIP is the insertion point at which the body code should be
+ /// placed.
+ /// \param DeallocIPs is the list of insertion points where explicit
+ /// deallocations, if needed, should be placed.
/// \return an error, if any were triggered during execution.
using BodyGenCallbackTy =
- function_ref<Error(InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
+ function_ref<Error(InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs)>;
// This is created primarily for sections construct as llvm::function_ref
// (BodyGenCallbackTy) is not storable (as described in the comments of
@@ -621,7 +623,8 @@ class OpenMPIRBuilder {
///
/// \return an error, if any were triggered during execution.
using StorableBodyGenCallbackTy =
- std::function<Error(InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
+ std::function<Error(InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs)>;
/// Callback type for loop body code generation.
///
@@ -715,7 +718,9 @@ class OpenMPIRBuilder {
/// Generator for '#omp parallel'
///
/// \param Loc The insert and source location description.
- /// \param AllocaIP The insertion points to be used for alloca instructions.
+ /// \param AllocIP The insertion point to be used for allocations.
+ /// \param DeallocIPs The insertion points to be used for explicit
+ /// deallocations, if needed.
/// \param BodyGenCB Callback that will generate the region code.
/// \param PrivCB Callback to copy a given variable (think copy constructor).
/// \param FiniCB Callback to finalize variable copies.
@@ -726,10 +731,10 @@ class OpenMPIRBuilder {
///
/// \returns The insertion position *after* the parallel.
LLVM_ABI InsertPointOrErrorTy createParallel(
- const LocationDescription &Loc, InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB, PrivatizeCallbackTy PrivCB,
- FinalizeCallbackTy FiniCB, Value *IfCondition, Value *NumThreads,
- omp::ProcBindKind ProcBind, bool IsCancellable);
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ ArrayRef<InsertPointTy> DeallocIPs, BodyGenCallbackTy BodyGenCB,
+ PrivatizeCallbackTy PrivCB, FinalizeCallbackTy FiniCB, Value *IfCondition,
+ Value *NumThreads, omp::ProcBindKind ProcBind, bool IsCancellable);
/// Generator for the control flow structure of an OpenMP canonical loop.
///
@@ -1347,7 +1352,9 @@ class OpenMPIRBuilder {
/// Generator for `#omp task`
///
/// \param Loc The location where the task construct was encountered.
- /// \param AllocaIP The insertion point to be used for alloca instructions.
+ /// \param AllocIP The insertion point to be used for allocations.
+ /// \param DeallocIPs The insertion points to be used for explicit
+ /// deallocations, if needed.
/// \param BodyGenCB Callback that will generate the region code.
/// \param Tied True if the task is tied, false if the task is untied.
/// \param Final i1 value which is `true` if the task is final, `false` if the
@@ -1363,21 +1370,23 @@ class OpenMPIRBuilder {
/// \param Mergeable If the given task is `mergeable`
/// \param priority `priority-value' specifies the execution order of the
/// tasks that is generated by the construct
- LLVM_ABI InsertPointOrErrorTy
- createTask(const LocationDescription &Loc, InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB, bool Tied = true,
- Value *Final = nullptr, Value *IfCondition = nullptr,
- SmallVector<DependData> Dependencies = {}, bool Mergeable = false,
- Value *EventHandle = nullptr, Value *Priority = nullptr);
+ LLVM_ABI InsertPointOrErrorTy createTask(
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ ArrayRef<InsertPointTy> DeallocIPs, BodyGenCallbackTy BodyGenCB,
+ bool Tied = true, Value *Final = nullptr, Value *IfCondition = nullptr,
+ SmallVector<DependData> Dependencies = {}, bool Mergeable = false,
+ Value *EventHandle = nullptr, Value *Priority = nullptr);
/// Generator for the taskgroup construct
///
/// \param Loc The location where the taskgroup construct was encountered.
- /// \param AllocaIP The insertion point to be used for alloca instructions.
+ /// \param AllocIP The insertion point to be used for allocations.
+ /// \param DeallocIPs The insertion point to be used for explicit deallocation
+ /// instructions, if needed.
/// \param BodyGenCB Callback that will generate the region code.
- LLVM_ABI InsertPointOrErrorTy createTaskgroup(const LocationDescription &Loc,
- InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB);
+ LLVM_ABI InsertPointOrErrorTy createTaskgroup(
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ ArrayRef<InsertPointTy> DeallocIPs, BodyGenCallbackTy BodyGenCB);
using FileIdentifierInfoCallbackTy =
std::function<std::tuple<std::string, uint64_t>()>;
@@ -2246,7 +2255,8 @@ class OpenMPIRBuilder {
struct OutlineInfo {
using PostOutlineCBTy = std::function<void(Function &)>;
PostOutlineCBTy PostOutlineCB;
- BasicBlock *EntryBB, *ExitBB, *OuterAllocaBB;
+ BasicBlock *EntryBB, *ExitBB, *OuterAllocBB;
+ SmallVector<BasicBlock *> OuterDeallocBBs;
SmallVector<Value *, 2> ExcludeArgsFromAggregate;
LLVM_ABI virtual ~OutlineInfo() = default;
@@ -2319,7 +2329,8 @@ class OpenMPIRBuilder {
/// \return an error, if any were triggered during execution.
LLVM_ABI Error emitIfClause(Value *Cond, BodyGenCallbackTy ThenGen,
BodyGenCallbackTy ElseGen,
- InsertPointTy AllocaIP = {});
+ InsertPointTy AllocIP = {},
+ ArrayRef<InsertPointTy> DeallocIPs = {});
/// Create the global variable holding the offload mappings information.
LLVM_ABI GlobalVariable *
@@ -2874,11 +2885,13 @@ class OpenMPIRBuilder {
/// Generator for `#omp distribute`
///
/// \param Loc The location where the distribute construct was encountered.
- /// \param AllocaIP The insertion points to be used for alloca instructions.
+ /// \param AllocIP The insertion point to be used for allocations.
+ /// \param DeallocIPs The insertion points to be used for explicit
+ /// deallocations, if needed.
/// \param BodyGenCB Callback that will generate the region code.
- LLVM_ABI InsertPointOrErrorTy createDistribute(const LocationDescription &Loc,
- InsertPointTy AllocaIP,
- BodyGenCallbackTy BodyGenCB);
+ LLVM_ABI InsertPointOrErrorTy createDistribute(
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ ArrayRef<InsertPointTy> DeallocIPs, BodyGenCallbackTy BodyGenCB);
/// Generate conditional branch and relevant BasicBlocks through which private
/// threads copy the 'copyin' variables from Master copy to threadprivate
@@ -3206,9 +3219,11 @@ class OpenMPIRBuilder {
/// Generator for '#omp target data'
///
/// \param Loc The location where the target data construct was encountered.
- /// \param AllocaIP The insertion points to be used for alloca instructions.
+ /// \param AllocIP The insertion points to be used for allocations.
/// \param CodeGenIP The insertion point at which the target directive code
/// should be placed.
+ /// \param DeallocIPs The insertion points at which explicit deallocations
+ /// should be placed, if needed.
/// \param IsBegin If true then emits begin mapper call otherwise emits
/// end mapper call.
/// \param DeviceID Stores the DeviceID from the device clause.
@@ -3221,10 +3236,10 @@ class OpenMPIRBuilder {
/// \param DeviceAddrCB Optional callback to generate code related to
/// use_device_ptr and use_device_addr.
LLVM_ABI InsertPointOrErrorTy createTargetData(
- const LocationDescription &Loc, InsertPointTy AllocaIP,
- InsertPointTy CodeGenIP, Value *DeviceID, Value *IfCond,
- TargetDataInfo &Info, GenMapInfoCallbackTy GenMapInfoCB,
- CustomMapperCallbackTy CustomMapperCB,
+ const LocationDescription &Loc, InsertPointTy AllocIP,
+ InsertPointTy CodeGenIP, ArrayRef<InsertPointTy> DeallocIPs,
+ Value *DeviceID, Value *IfCond, TargetDataInfo &Info,
+ GenMapInfoCallbackTy GenMapInfoCB, CustomMapperCallbackTy CustomMapperCB,
omp::RuntimeFunction *MapperFunc = nullptr,
function_ref<InsertPointOrErrorTy(InsertPointTy CodeGenIP,
BodyGenTy BodyGenType)>
@@ -3233,7 +3248,8 @@ class OpenMPIRBuilder {
Value *SrcLocInfo = nullptr);
using TargetBodyGenCallbackTy = function_ref<InsertPointOrErrorTy(
- InsertPointTy AllocaIP, InsertPointTy CodeGenIP)>;
+ InsertPointTy AllocIP, InsertPointTy CodeGenIP,
+ ArrayRef<InsertPointTy> DeallocIPs)>;
using TargetGenArgAccessorsCallbackTy = function_ref<InsertPointOrErrorTy(
Argument &Arg, Value *Input, Value *&RetVal, InsertPointTy AllocaIP,
@@ -3245,6 +3261,8 @@ class OpenMPIRBuilder {
/// \param IsOffloadEntry whether it is an offload entry.
/// \param CodeGenIP The insertion point where the call to the outlined
/// function should be emitted.
+ /// \param DeallocIPs The insertion points at which explicit deallocations
+ /// should be placed, if needed.
/// \param Info Stores all information realted to the Target directive.
/// \param EntryInfo The entry information about the function.
/// \param DefaultAttrs Structure containing the default attributes, including
@@ -3265,8 +3283,9 @@ class OpenMPIRBuilder {
/// not.
LLVM_ABI InsertPointOrErrorTy createTarget(
const LocationDescription &Loc, bool IsOffloadEntry,
- OpenMPIRBuilder::InsertPointTy AllocaIP,
- OpenMPIRBuilder::InsertPointTy CodeGenIP, TargetDataInfo &Info,
+ OpenMPIRBuilder::InsertPointTy AllocIP,
+ Open...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
In this patch, some OMPIRBuilder codegen functions and callbacks are updated to work with arrays of deallocation insertion points. The purpose of this is to enable the replacement of `alloca`s with other types of allocations that require explicit deallocations in a way that makes it possible for `CodeExtractor` instances created during OMPIRBuilder finalization to also use them. The OpenMP to LLVM IR MLIR translation pass is updated to properly store and forward deallocation points together with their matching allocation point to the OMPIRBuilder. Currently, only the `DeviceSharedMemCodeExtractor` uses this feature to get the `CodeExtractor` to use device shared memory for intermediate allocations when outlining a parallel region inside of a Generic kernel (code path that is only used by Flang via MLIR, currently). However, long term this might also be useful to refactor finalization of variables with destructors, potentially reducing the use of callbacks and simplifying privatization and reductions. Instead of a single deallocation point, lists of those are used. This is to cover cases where there are multiple exit blocks originating from a single entry. If an allocation needing explicit deallocation is placed in the entry block of such cases, it would need to be deallocated before each of the exits.
skatrak
force-pushed
the
users/skatrak/flang-generic-07-dealloc-point
branch
from
e06d69e to
bdd6b36
Compare
This was referenced Aug 21, 2025
Meinersbur
reviewed
Aug 22, 2025
There was a problem hiding this comment.
Instead of a single deallocation point, lists of those are used. This is to cover cases where there are multiple exit blocks originating from a single entry. If an allocation needing explicit deallocation is placed in the entry block of such cases, it would need to be deallocated before each of the exits.
Clang uses a pattern that uses only a single block for multiple exits: Assing each exiting block an unique number, jump to a single BB that impements the dtor, then switch to the successor of the exiting block according the that unique number. I assume this is done to reduce code size, LLVM JumpThreading pass can duplicate that block again depending on whether we optimize for size.
| IfCond, NumThreads, ProcBind, S.hasCancel())); | ||
| llvm::OpenMPIRBuilder::InsertPointTy AfterIP = | ||
| cantFail(OMPBuilder.createParallel( | ||
| Builder, AllocaIP, /*DeallocIPs=*/{}, BodyGenCB, PrivCB, FiniCB, |
There was a problem hiding this comment.
What will happen with Clang if deallocation is needed?
There was a problem hiding this comment.
At the moment, the only code path that uses the DeallocIPs is the one outlining the parallel region for a target device, so this call in Clang doesn't use it.
If at some point explicit allocations were introduced for the host at the AllocaIP and the list here remained empty, the default deallocation point would be the exit block of the parallel region inside of the outlined function (see the CodeExtractor when DeallocationBlocks.empty(), for reference). That could potentially be wrong, if the AllocaIP happened to be located outside of the outlined region.
| OI->EntryBB = PRegEntryBB; | ||
| OI->ExitBB = PRegExitBB; | ||
| OI->OuterDeallocBBs.reserve(OuterDeallocIPs.size()); | ||
| for (InsertPointTy DeallocIP : OuterDeallocIPs) | ||
| OI->OuterDeallocBBs.push_back(DeallocIP.getBlock()); |
There was a problem hiding this comment.
Since we are passing an InsertionPoint, shouldn't we pass the exact position and not just the block assuming that getFirstInsertionPt() what the DeallocIP?
There was a problem hiding this comment.
I'd agree with that. The reason I didn't do it was that we already discard the specific insert point information for the allocation point, and just pass the block.
Since InsertPointTy is defined in IRBuilder, we'd either have to move it or copy it into the CodeExtractor or store both block and iterator separately for each of these points in the CodeExtractor. Not sure if this is worth doing here, but let me know if you'd like to see that change.
There was a problem hiding this comment.
Since InsertPointTy is defined in IRBuilder, we'd either have to move it or copy it into the CodeExtractor or store both block and iterator separately for each of these points in the CodeExtractor.
Is it just to avoid #include <llvm/IR/IRBuilder.h> in CodeExtractor.h?
The reason I didn't do it was that we already discard the specific insert point information for the allocation point, and just pass the block.
If only the block is relevant, a reasonable API passes only the block. Since there is precedence/symmetry, consider adding an assertion that checks that the IP is pointing to getFirstInsertionPt() or at least add a warning about this.
skatrak
commented
Aug 26, 2025
There was a problem hiding this comment.
Thank you Michael for the review!
Clang uses a pattern that uses only a single block for multiple exits
I'm not too familiar with this, but in any case there is at least one situation in Flang where multiple exits are used. If you look at findAllocInsertPoints, there you can see that whenever there are no eligible OpenMPAllocStackFrames, it will use the function entry block as allocation insert point. In that case, we need to make every exit out of the function a deallocation point because there is nothing to prevent that function from having multiple returns and we have no control in the OpenMP dialect over how the body of a function as a whole gets translated to LLVM IR.
So, I'd say the vast majority of the time it will be empty or a single point, but we need to support multiple exits for at least that situation.
| IfCond, NumThreads, ProcBind, S.hasCancel())); | ||
| llvm::OpenMPIRBuilder::InsertPointTy AfterIP = | ||
| cantFail(OMPBuilder.createParallel( | ||
| Builder, AllocaIP, /*DeallocIPs=*/{}, BodyGenCB, PrivCB, FiniCB, |
There was a problem hiding this comment.
At the moment, the only code path that uses the DeallocIPs is the one outlining the parallel region for a target device, so this call in Clang doesn't use it.
If at some point explicit allocations were introduced for the host at the AllocaIP and the list here remained empty, the default deallocation point would be the exit block of the parallel region inside of the outlined function (see the CodeExtractor when DeallocationBlocks.empty(), for reference). That could potentially be wrong, if the AllocaIP happened to be located outside of the outlined region.
| OI->EntryBB = PRegEntryBB; | ||
| OI->ExitBB = PRegExitBB; | ||
| OI->OuterDeallocBBs.reserve(OuterDeallocIPs.size()); | ||
| for (InsertPointTy DeallocIP : OuterDeallocIPs) | ||
| OI->OuterDeallocBBs.push_back(DeallocIP.getBlock()); |
There was a problem hiding this comment.
I'd agree with that. The reason I didn't do it was that we already discard the specific insert point information for the allocation point, and just pass the block.
Since InsertPointTy is defined in IRBuilder, we'd either have to move it or copy it into the CodeExtractor or store both block and iterator separately for each of these points in the CodeExtractor. Not sure if this is worth doing here, but let me know if you'd like to see that change.
tblah
reviewed
Aug 26, 2025
There was a problem hiding this comment.
Could this lead to mixing up allocas and heap allocations at the same insertion point? It would be much better if all allocas came first and then all heap allocations came second - IIRC this is why things were named to refer to alloca insertion points - heap allocations were treated more like regular code. Is there a later LLVM pass that is smart enough to move all the allocas to the start of the block?
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.
In this patch, some OMPIRBuilder codegen functions and callbacks are updated to work with arrays of deallocation insertion points. The purpose of this is to enable the replacement of
allocas with other types of allocations that require explicit deallocations in a way that makes it possible forCodeExtractorinstances created during OMPIRBuilder finalization to also use them.The OpenMP to LLVM IR MLIR translation pass is updated to properly store and forward deallocation points together with their matching allocation point to the OMPIRBuilder.
Currently, only the
DeviceSharedMemCodeExtractoruses this feature to get theCodeExtractorto use device shared memory for intermediate allocations when outlining a parallel region inside of a Generic kernel (code path that is only used by Flang via MLIR, currently). However, long term this might also be useful to refactor finalization of variables with destructors, potentially reducing the use of callbacks and simplifying privatization and reductions.Instead of a single deallocation point, lists of those are used. This is to cover cases where there are multiple exit blocks originating from a single entry. If an allocation needing explicit deallocation is placed in the entry block of such cases, it would need to be deallocated before each of the exits.