Skip to content

[OpenMP][OMPIRBuilder] Error propagation across callbacks #112533

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

Merged
skatrak merged 4 commits into from
Oct 25, 2024
Merged

[OpenMP][OMPIRBuilder] Error propagation across callbacks #112533

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
568964e
[OpenMP][OMPIRBuilder] Error propagation across callbacks
skatrak Oct 23, 2024
83091fa
Fix formatting issues
skatrak Oct 23, 2024
15c1f84
Fix misleading error message
skatrak Oct 24, 2024
66579ef
Merge branch 'main' into omp-mlir-llvm-error-handling
skatrak Oct 25, 2024
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
32 changes: 22 additions & 10 deletions clang/lib/CodeGen/CGOpenMPRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1192,6 +1192,7 @@ struct PushAndPopStackRAII {
CodeGenFunction::JumpDest Dest =
CGF.getOMPCancelDestination(OMPD_parallel);
CGF.EmitBranchThroughCleanup(Dest);
return llvm::Error::success();
};

// TODO: Remove this once we emit parallel regions through the
Expand Down Expand Up @@ -2331,8 +2332,11 @@ void CGOpenMPRuntime::emitBarrierCall(CodeGenFunction &CGF, SourceLocation Loc,
auto *OMPRegionInfo =
dyn_cast_or_null<CGOpenMPRegionInfo>(CGF.CapturedStmtInfo);
if (CGF.CGM.getLangOpts().OpenMPIRBuilder) {
CGF.Builder.restoreIP(OMPBuilder.createBarrier(
CGF.Builder, Kind, ForceSimpleCall, EmitChecks));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createBarrier(CGF.Builder, Kind, ForceSimpleCall,
EmitChecks);
assert(AfterIP && "unexpected error creating barrier");
Copy link
Member

@Meinersbur Meinersbur Nov 11, 2024

Choose a reason for hiding this comment

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

[serious] @skatrak assert is active by LLVM_ENABLED_ASSERTS, but llvm::Error/llvm::Expected uses LLVM_ABI_BREAKING_CHECKS. In the configuration -DLLVM_ENABLED_ASSERTS=OFF -DLLVM_ABI_BREAKING_CHECKS=FORCE_ON the error check is not done, but then crashes unconditional to about unchecked errors.

Copy link
Member Author

@skatrak skatrak Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I guess then these asserts should be replaced with if (!AfterIP) llvm_unreachable(...), or is there a better way to deal with this?

Copy link
Member Author

@skatrak skatrak Nov 12, 2024

Choose a reason for hiding this comment

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

Here's the PR to address this issue: #115863. There was indeed a cleaner solution to this.

CGF.Builder.restoreIP(*AfterIP);
return;
}

Expand Down Expand Up @@ -5928,8 +5932,10 @@ void CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(
return CGF.GenerateOpenMPCapturedStmtFunction(CS, D.getBeginLoc());
};

OMPBuilder.emitTargetRegionFunction(EntryInfo, GenerateOutlinedFunction,
IsOffloadEntry, OutlinedFn, OutlinedFnID);
llvm::Error Err = OMPBuilder.emitTargetRegionFunction(
EntryInfo, GenerateOutlinedFunction, IsOffloadEntry, OutlinedFn,
OutlinedFnID);
assert(!Err && "unexpected error creating target region");

if (!OutlinedFn)
return;
Expand Down Expand Up @@ -9671,9 +9677,12 @@ static void emitTargetCallKernelLaunch(
NumTargetItems, RTArgs, NumIterations, NumTeams, NumThreads,
DynCGGroupMem, HasNoWait);

CGF.Builder.restoreIP(OMPRuntime->getOMPBuilder().emitKernelLaunch(
CGF.Builder, OutlinedFnID, EmitTargetCallFallbackCB, Args, DeviceID,
RTLoc, AllocaIP));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPRuntime->getOMPBuilder().emitKernelLaunch(
CGF.Builder, OutlinedFnID, EmitTargetCallFallbackCB, Args, DeviceID,
RTLoc, AllocaIP);
assert(AfterIP && "unexpected error creating kernel launch");
CGF.Builder.restoreIP(*AfterIP);
};

if (RequiresOuterTask)
Expand Down Expand Up @@ -10350,9 +10359,12 @@ void CGOpenMPRuntime::emitTargetDataCalls(
InsertPointTy CodeGenIP(CGF.Builder.GetInsertBlock(),
CGF.Builder.GetInsertPoint());
llvm::OpenMPIRBuilder::LocationDescription OmpLoc(CodeGenIP);
CGF.Builder.restoreIP(OMPBuilder.createTargetData(
OmpLoc, AllocaIP, CodeGenIP, DeviceID, IfCondVal, Info, GenMapInfoCB,
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createTargetData(
OmpLoc, AllocaIP, CodeGenIP, DeviceID, IfCondVal, Info, GenMapInfoCB,
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc);
assert(AfterIP && "unexpected error creating target data");
CGF.Builder.restoreIP(*AfterIP);
}

void CGOpenMPRuntime::emitTargetDataStandAloneCall(
Expand Down
13 changes: 8 additions & 5 deletions clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1753,11 +1753,14 @@ void CGOpenMPRuntimeGPU::emitReduction(
Idx++;
}

CGF.Builder.restoreIP(OMPBuilder.createReductionsGPU(
OmpLoc, AllocaIP, CodeGenIP, ReductionInfos, false, TeamsReduction,
DistributeReduction, llvm::OpenMPIRBuilder::ReductionGenCBKind::Clang,
CGF.getTarget().getGridValue(), C.getLangOpts().OpenMPCUDAReductionBufNum,
RTLoc));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createReductionsGPU(
OmpLoc, AllocaIP, CodeGenIP, ReductionInfos, false, TeamsReduction,
DistributeReduction, llvm::OpenMPIRBuilder::ReductionGenCBKind::Clang,
CGF.getTarget().getGridValue(),
C.getLangOpts().OpenMPCUDAReductionBufNum, RTLoc);
assert(AfterIP && "unexpected error creating GPU reductions");
CGF.Builder.restoreIP(*AfterIP);
return;
}

Expand Down
90 changes: 67 additions & 23 deletions clang/lib/CodeGen/CGStmtOpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1809,6 +1809,7 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
// thus calls destructors etc.
auto FiniCB = [this](InsertPointTy IP) {
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
return llvm::Error::success();
};

// Privatization callback that performs appropriate action for
Expand All @@ -1831,15 +1832,18 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPOutlinedRegionBody(
*this, ParallelRegionBodyStmt, AllocaIP, CodeGenIP, "parallel");
return llvm::Error::success();
};

CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
Builder.restoreIP(
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createParallel(Builder, AllocaIP, BodyGenCB, PrivCB, FiniCB,
IfCond, NumThreads, ProcBind, S.hasCancel()));
IfCond, NumThreads, ProcBind, S.hasCancel());
assert(AfterIP && "unexpected error creating parallel");
Builder.restoreIP(*AfterIP);
return;
}

Expand Down Expand Up @@ -2128,9 +2132,13 @@ void CodeGenFunction::EmitOMPCanonicalLoop(const OMPCanonicalLoop *S) {

RunCleanupsScope BodyScope(*this);
EmitStmt(BodyStmt);
return llvm::Error::success();
};
llvm::CanonicalLoopInfo *CL =

llvm::Expected<llvm::CanonicalLoopInfo *> Result =
OMPBuilder.createCanonicalLoop(Builder, BodyGen, DistVal);
assert(Result && "unexpected error creating parallel");
llvm::CanonicalLoopInfo *CL = *Result;

// Finish up the loop.
Builder.restoreIP(CL->getAfterIP());
Expand Down Expand Up @@ -4016,11 +4024,13 @@ static void emitOMPForDirective(const OMPLoopDirective &S, CodeGenFunction &CGF,
CGM.getOpenMPRuntime().getOMPBuilder();
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
CGF.AllocaInsertPt->getParent(), CGF.AllocaInsertPt->getIterator());
OMPBuilder.applyWorkshareLoop(
CGF.Builder.getCurrentDebugLocation(), CLI, AllocaIP, NeedsBarrier,
SchedKind, ChunkSize, /*HasSimdModifier=*/false,
/*HasMonotonicModifier=*/false, /*HasNonmonotonicModifier=*/false,
/*HasOrderedClause=*/false);
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.applyWorkshareLoop(
CGF.Builder.getCurrentDebugLocation(), CLI, AllocaIP,
NeedsBarrier, SchedKind, ChunkSize, /*HasSimdModifier=*/false,
/*HasMonotonicModifier=*/false, /*HasNonmonotonicModifier=*/false,
/*HasOrderedClause=*/false);
assert(AfterIP && "unexpected error creating workshare loop");
return;
}

Expand Down Expand Up @@ -4257,6 +4267,7 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {

auto FiniCB = [this](InsertPointTy IP) {
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
return llvm::Error::success();
};

const CapturedStmt *ICS = S.getInnermostCapturedStmt();
Expand All @@ -4269,6 +4280,7 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, SubStmt, AllocaIP, CodeGenIP, "section");
return llvm::Error::success();
};
SectionCBVector.push_back(SectionCB);
}
Expand All @@ -4277,6 +4289,7 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, CapturedStmt, AllocaIP, CodeGenIP, "section");
return llvm::Error::success();
};
SectionCBVector.push_back(SectionCB);
}
Expand All @@ -4298,9 +4311,12 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
Builder.restoreIP(OMPBuilder.createSections(
Builder, AllocaIP, SectionCBVector, PrivCB, FiniCB, S.hasCancel(),
S.getSingleClause<OMPNowaitClause>()));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createSections(Builder, AllocaIP, SectionCBVector, PrivCB,
FiniCB, S.hasCancel(),
S.getSingleClause<OMPNowaitClause>());
assert(AfterIP && "unexpected error creating sections");
Builder.restoreIP(*AfterIP);
return;
}
{
Expand All @@ -4326,17 +4342,22 @@ void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {
const Stmt *SectionRegionBodyStmt = S.getAssociatedStmt();
auto FiniCB = [this](InsertPointTy IP) {
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
return llvm::Error::success();
};

auto BodyGenCB = [SectionRegionBodyStmt, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, SectionRegionBodyStmt, AllocaIP, CodeGenIP, "section");
return llvm::Error::success();
};

LexicalScope Scope(*this, S.getSourceRange());
EmitStopPoint(&S);
Builder.restoreIP(OMPBuilder.createSection(Builder, BodyGenCB, FiniCB));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createSection(Builder, BodyGenCB, FiniCB);
assert(AfterIP && "unexpected error creating section");
Builder.restoreIP(*AfterIP);

return;
}
Expand Down Expand Up @@ -4407,17 +4428,22 @@ void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {

auto FiniCB = [this](InsertPointTy IP) {
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
return llvm::Error::success();
};

auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, MasterRegionBodyStmt, AllocaIP, CodeGenIP, "master");
return llvm::Error::success();
};

LexicalScope Scope(*this, S.getSourceRange());
EmitStopPoint(&S);
Builder.restoreIP(OMPBuilder.createMaster(Builder, BodyGenCB, FiniCB));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createMaster(Builder, BodyGenCB, FiniCB);
assert(AfterIP && "unexpected error creating master");
Builder.restoreIP(*AfterIP);

return;
}
Expand Down Expand Up @@ -4453,18 +4479,22 @@ void CodeGenFunction::EmitOMPMaskedDirective(const OMPMaskedDirective &S) {

auto FiniCB = [this](InsertPointTy IP) {
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
return llvm::Error::success();
};

auto BodyGenCB = [MaskedRegionBodyStmt, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, MaskedRegionBodyStmt, AllocaIP, CodeGenIP, "masked");
return llvm::Error::success();
};

LexicalScope Scope(*this, S.getSourceRange());
EmitStopPoint(&S);
Builder.restoreIP(
OMPBuilder.createMasked(Builder, BodyGenCB, FiniCB, FilterVal));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createMasked(Builder, BodyGenCB, FiniCB, FilterVal);
assert(AfterIP && "unexpected error creating masked");
Builder.restoreIP(*AfterIP);

return;
}
Expand Down Expand Up @@ -4493,19 +4523,23 @@ void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {

auto FiniCB = [this](InsertPointTy IP) {
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
return llvm::Error::success();
};

auto BodyGenCB = [CriticalRegionBodyStmt, this](InsertPointTy AllocaIP,
InsertPointTy CodeGenIP) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, CriticalRegionBodyStmt, AllocaIP, CodeGenIP, "critical");
return llvm::Error::success();
};

LexicalScope Scope(*this, S.getSourceRange());
EmitStopPoint(&S);
Builder.restoreIP(OMPBuilder.createCritical(
Builder, BodyGenCB, FiniCB, S.getDirectiveName().getAsString(),
HintInst));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createCritical(Builder, BodyGenCB, FiniCB,
S.getDirectiveName().getAsString(), HintInst);
assert(AfterIP && "unexpected error creating critical");
Builder.restoreIP(*AfterIP);

return;
}
Expand Down Expand Up @@ -5464,11 +5498,15 @@ void CodeGenFunction::EmitOMPTaskgroupDirective(
InsertPointTy CodeGenIP) {
Builder.restoreIP(CodeGenIP);
EmitStmt(S.getInnermostCapturedStmt()->getCapturedStmt());
return llvm::Error::success();
};
CodeGenFunction::CGCapturedStmtInfo CapStmtInfo;
if (!CapturedStmtInfo)
CapturedStmtInfo = &CapStmtInfo;
Builder.restoreIP(OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB);
assert(AfterIP && "unexpected error creating taskgroup");
Builder.restoreIP(*AfterIP);
return;
}
auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
Expand Down Expand Up @@ -6041,6 +6079,7 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {

auto FiniCB = [this](InsertPointTy IP) {
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
return llvm::Error::success();
};

auto BodyGenCB = [&S, C, this](InsertPointTy AllocaIP,
Expand All @@ -6064,11 +6103,14 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
*this, CS->getCapturedStmt(), AllocaIP, CodeGenIP, "ordered");
}
return llvm::Error::success();
};

OMPLexicalScope Scope(*this, S, OMPD_unknown);
Builder.restoreIP(
OMPBuilder.createOrderedThreadsSimd(Builder, BodyGenCB, FiniCB, !C));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createOrderedThreadsSimd(Builder, BodyGenCB, FiniCB, !C);
assert(AfterIP && "unexpected error creating ordered");
Builder.restoreIP(*AfterIP);
}
return;
}
Expand Down Expand Up @@ -7344,8 +7386,10 @@ void CodeGenFunction::EmitOMPCancelDirective(const OMPCancelDirective &S) {
if (IfCond)
IfCondition = EmitScalarExpr(IfCond,
/*IgnoreResultAssign=*/true);
return Builder.restoreIP(
OMPBuilder.createCancel(Builder, IfCondition, S.getCancelRegion()));
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createCancel(Builder, IfCondition, S.getCancelRegion());
assert(AfterIP && "unexpected error creating cancel");
return Builder.restoreIP(*AfterIP);
}
}

Expand Down
Loading
Loading