Skip to content

Commit

Permalink
[OpenMP][OMPIRBuilder] Error propagation across callbacks (#112533)
Browse files Browse the repository at this point in the history 
This patch implements an approach to communicate errors between the
OMPIRBuilder and its users. It introduces `llvm::Error` and
`llvm::Expected` objects to replace the values returned by callbacks
passed to `OMPIRBuilder` codegen functions. These functions then check
the result for errors when callbacks are called and forward them back to
the caller, which has the flexibility to recover, exit cleanly or dump a
stack trace.

This prevents a failed callback to leave the IR in an invalid state and
still continue the codegen process, triggering unrelated assertions or
segmentation faults. In the case of MLIR to LLVM IR translation of the
'omp' dialect, this change results in the compiler emitting errors and
exiting early instead of triggering a crash for not-yet-implemented
errors. The behavior in Clang and openmp-opt stays unchanged, since
callbacks will continue always returning 'success'.
  • Loading branch information
@skatrak
skatrak authored Oct 25, 2024
1 parent 86d65ae commit d87964d
Show file tree
Hide file tree
Showing 8 changed files with 1,467 additions and 858 deletions.
32 changes: 22 additions & 10 deletions clang/lib/CodeGen/CGOpenMPRuntime.cpp
View file
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");
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 @@ -9670,9 +9676,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 @@ -10349,9 +10358,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
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
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 canonical loop");
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

0 comments on commit d87964d

Please sign in to comment.