Skip to content

[OMPIRBuilder] Add support for explicit deallocation points #154752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: users/skatrak/flang-generic-06-openmp-opt
Choose a base branch
from
Open

[OMPIRBuilder] Add support for explicit deallocation points #154752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CGOpenMPRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10923,8 +10923,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);
}
Expand Down
77 changes: 43 additions & 34 deletions clang/lib/CodeGen/CGStmtOpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1886,20 +1886,21 @@ 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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we gain very little from renaming AllocaIP -> AllocIP. But this change is bloating the overall diff quite a lot, and also unnecessarily taints the git history for these.

I'd suggest reverting this.

ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPOutlinedRegionBody(
*this, ParallelRegionBodyStmt, AllocaIP, CodeGenIP, "parallel");
*this, ParallelRegionBodyStmt, AllocIP, CodeGenIP, "parallel");
return llvm::Error::success();
};

CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen with Clang if deallocation is needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

IfCond, NumThreads, ProcBind, S.hasCancel()));
Builder.restoreIP(AfterIP);
return;
}
Expand Down Expand Up @@ -4427,21 +4428,23 @@ 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) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, SubStmt, AllocaIP, CodeGenIP, "section");
auto SectionCB = [this, SubStmt](InsertPointTy AllocIP,
InsertPointTy CodeGenIP,
ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(*this, SubStmt, AllocIP,
CodeGenIP, "section");
return llvm::Error::success();
};
SectionCBVector.push_back(SectionCB);
}
} else {
auto SectionCB = [this, CapturedStmt](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, CapturedStmt, AllocaIP, CodeGenIP, "section");
return llvm::Error::success();
};
auto SectionCB =
[this, CapturedStmt](InsertPointTy AllocIP, InsertPointTy CodeGenIP,
ArrayRef<InsertPointTy> DeallocIPs) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, CapturedStmt, AllocIP, CodeGenIP, "section");
return llvm::Error::success();
};
SectionCBVector.push_back(SectionCB);
}

Expand Down Expand Up @@ -4495,10 +4498,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();
};

Expand Down Expand Up @@ -4580,10 +4584,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();
};

Expand Down Expand Up @@ -4630,10 +4635,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();
};

Expand Down Expand Up @@ -4673,10 +4679,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();
};

Expand Down Expand Up @@ -5643,8 +5650,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();
Expand All @@ -5653,7 +5660,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;
}
Expand Down Expand Up @@ -6233,8 +6241,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();
Expand All @@ -6251,7 +6260,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();
};
Expand Down
99 changes: 59 additions & 40 deletions llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,17 +612,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
Expand All @@ -631,7 +633,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.
///
Expand Down Expand Up @@ -725,7 +728,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.
Expand All @@ -736,10 +741,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.
///
Expand Down Expand Up @@ -1363,7 +1368,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
Expand All @@ -1379,21 +1386,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>()>;
Expand Down Expand Up @@ -2262,7 +2271,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;
Expand Down Expand Up @@ -2335,7 +2345,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 *
Expand Down Expand Up @@ -2891,11 +2902,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
Expand Down Expand Up @@ -3223,9 +3236,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.
Expand All @@ -3238,10 +3253,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)>
Expand All @@ -3250,7 +3265,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,
Expand All @@ -3262,6 +3278,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
Expand All @@ -3282,8 +3300,9 @@ class OpenMPIRBuilder {
/// not.
LLVM_ABI InsertPointOrErrorTy createTarget(
const LocationDescription &Loc, bool IsOffloadEntry,
OpenMPIRBuilder::InsertPointTy AllocaIP,
OpenMPIRBuilder::InsertPointTy CodeGenIP, TargetDataInfo &Info,
OpenMPIRBuilder::InsertPointTy AllocIP,
OpenMPIRBuilder::InsertPointTy CodeGenIP,
ArrayRef<InsertPointTy> DeallocIPs, TargetDataInfo &Info,
TargetRegionEntryInfo &EntryInfo,
const TargetKernelDefaultAttrs &DefaultAttrs,
const TargetKernelRuntimeAttrs &RuntimeAttrs, Value *IfCond,
Expand Down
Loading