Skip to content

Commit

Permalink
[OpenMP][OMPIRBuilder] Handle non-failing calls properly (#115863)
Browse files Browse the repository at this point in the history 
The preprocessor definition used to enable asserts and the one that
`llvm::Error` and `llvm::Expected` use to ensure all created instances are
checked are not the same. By making these checks inside of an `assert` in cases
where errors are not expected, certain build configurations would trigger
runtime failures (e.g. `-DLLVM_ENABLE_ASSERTIONS=OFF
-DLLVM_UNREACHABLE_OPTIMIZE=ON`).

The `llvm::cantFail()` function, which was intended for this use case, is used
by this patch in place of `assert` to prevent these runtime failures. In tests,
new preprocessor definitions based on `ASSERT_THAT_EXPECTED` and
`EXPECT_THAT_EXPECTED` are used instead, to avoid silent failures in release
builds.
  • Loading branch information
@skatrak
skatrak authored Jan 9, 2025
1 parent 659cd2a commit b79ed87
Show file tree
Hide file tree
Showing 6 changed files with 524 additions and 499 deletions.
32 changes: 14 additions & 18 deletions clang/lib/CodeGen/CGOpenMPRuntime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2327,11 +2327,10 @@ void CGOpenMPRuntime::emitBarrierCall(CodeGenFunction &CGF, SourceLocation Loc,
auto *OMPRegionInfo =
dyn_cast_or_null<CGOpenMPRegionInfo>(CGF.CapturedStmtInfo);
if (CGF.CGM.getLangOpts().OpenMPIRBuilder) {
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createBarrier(CGF.Builder, Kind, ForceSimpleCall,
EmitChecks);
assert(AfterIP && "unexpected error creating barrier");
CGF.Builder.restoreIP(*AfterIP);
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
cantFail(OMPBuilder.createBarrier(CGF.Builder, Kind, ForceSimpleCall,
EmitChecks));
CGF.Builder.restoreIP(AfterIP);
return;
}

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

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

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

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

if (RequiresOuterTask)
Expand Down Expand Up @@ -10091,12 +10088,11 @@ void CGOpenMPRuntime::emitTargetDataCalls(
InsertPointTy CodeGenIP(CGF.Builder.GetInsertBlock(),
CGF.Builder.GetInsertPoint());
llvm::OpenMPIRBuilder::LocationDescription OmpLoc(CodeGenIP);
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createTargetData(
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
cantFail(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);
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc));
CGF.Builder.restoreIP(AfterIP);
}

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

llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createReductionsGPU(
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
cantFail(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);
C.getLangOpts().OpenMPCUDAReductionBufNum, RTLoc));
CGF.Builder.restoreIP(AfterIP);
return;
}

Expand Down
88 changes: 38 additions & 50 deletions clang/lib/CodeGen/CGStmtOpenMP.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1839,11 +1839,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
llvm::OpenMPIRBuilder::InsertPointTy AfterIP = cantFail(
OMPBuilder.createParallel(Builder, AllocaIP, BodyGenCB, PrivCB, FiniCB,
IfCond, NumThreads, ProcBind, S.hasCancel());
assert(AfterIP && "unexpected error creating parallel");
Builder.restoreIP(*AfterIP);
IfCond, NumThreads, ProcBind, S.hasCancel()));
Builder.restoreIP(AfterIP);
return;
}

Expand Down Expand Up @@ -2135,10 +2134,8 @@ void CodeGenFunction::EmitOMPCanonicalLoop(const OMPCanonicalLoop *S) {
return llvm::Error::success();
};

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

// Finish up the loop.
Builder.restoreIP(CL->getAfterIP());
Expand Down Expand Up @@ -4024,13 +4021,11 @@ static void emitOMPForDirective(const OMPLoopDirective &S, CodeGenFunction &CGF,
CGM.getOpenMPRuntime().getOMPBuilder();
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
CGF.AllocaInsertPt->getParent(), CGF.AllocaInsertPt->getIterator());
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");
cantFail(OMPBuilder.applyWorkshareLoop(
CGF.Builder.getCurrentDebugLocation(), CLI, AllocaIP, NeedsBarrier,
SchedKind, ChunkSize, /*HasSimdModifier=*/false,
/*HasMonotonicModifier=*/false, /*HasNonmonotonicModifier=*/false,
/*HasOrderedClause=*/false));
return;
}

Expand Down Expand Up @@ -4311,12 +4306,11 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
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);
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
cantFail(OMPBuilder.createSections(
Builder, AllocaIP, SectionCBVector, PrivCB, FiniCB, S.hasCancel(),
S.getSingleClause<OMPNowaitClause>()));
Builder.restoreIP(AfterIP);
return;
}
{
Expand Down Expand Up @@ -4354,10 +4348,9 @@ void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {

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

return;
}
Expand Down Expand Up @@ -4440,10 +4433,9 @@ void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {

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

return;
}
Expand Down Expand Up @@ -4491,10 +4483,9 @@ void CodeGenFunction::EmitOMPMaskedDirective(const OMPMaskedDirective &S) {

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

return;
}
Expand Down Expand Up @@ -4535,11 +4526,11 @@ void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {

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

return;
}
Expand Down Expand Up @@ -5503,10 +5494,9 @@ void CodeGenFunction::EmitOMPTaskgroupDirective(
CodeGenFunction::CGCapturedStmtInfo CapStmtInfo;
if (!CapturedStmtInfo)
CapturedStmtInfo = &CapStmtInfo;
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB);
assert(AfterIP && "unexpected error creating taskgroup");
Builder.restoreIP(*AfterIP);
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
cantFail(OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB));
Builder.restoreIP(AfterIP);
return;
}
auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
Expand Down Expand Up @@ -6109,10 +6099,9 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
};

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

Expand Down
32 changes: 12 additions & 20 deletions llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4243,23 +4243,19 @@ OpenMPIRBuilder::applyStaticChunkedWorkshareLoop(DebugLoc DL,
// Create outer "dispatch" loop for enumerating the chunks.
BasicBlock *DispatchEnter = splitBB(Builder, true);
Value *DispatchCounter;
Expected<CanonicalLoopInfo *> LoopResult = createCanonicalLoop(

// It is safe to assume this didn't return an error because the callback
// passed into createCanonicalLoop is the only possible error source, and it
// always returns success.
CanonicalLoopInfo *DispatchCLI = cantFail(createCanonicalLoop(
{Builder.saveIP(), DL},
[&](InsertPointTy BodyIP, Value *Counter) {
DispatchCounter = Counter;
return Error::success();
},
FirstChunkStart, CastedTripCount, NextChunkStride,
/*IsSigned=*/false, /*InclusiveStop=*/false, /*ComputeIP=*/{},
"dispatch");
if (!LoopResult) {
// It is safe to assume this didn't return an error because the callback
// passed into createCanonicalLoop is the only possible error source, and it
// always returns success. Need to still cast the result into bool to avoid
// runtime errors.
llvm_unreachable("unexpected error creating canonical loop");
}
CanonicalLoopInfo *DispatchCLI = *LoopResult;
"dispatch"));

// Remember the BasicBlocks of the dispatch loop we need, then invalidate to
// not have to preserve the canonical invariant.
Expand Down Expand Up @@ -6561,16 +6557,12 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTargetData(
};

bool RequiresOuterTargetTask = Info.HasNoWait;
if (!RequiresOuterTargetTask) {
Error Err = TaskBodyCB(/*DeviceID=*/nullptr, /*RTLoc=*/nullptr,
/*TargetTaskAllocaIP=*/{});
assert(!Err && "TaskBodyCB expected to succeed");
} else {
InsertPointOrErrorTy AfterIP =
emitTargetTask(TaskBodyCB, DeviceID, SrcLocInfo, AllocaIP,
/*Dependencies=*/{}, Info.HasNoWait);
assert(AfterIP && "TaskBodyCB expected to succeed");
}
if (!RequiresOuterTargetTask)
cantFail(TaskBodyCB(/*DeviceID=*/nullptr, /*RTLoc=*/nullptr,
/*TargetTaskAllocaIP=*/{}));
else
cantFail(emitTargetTask(TaskBodyCB, DeviceID, SrcLocInfo, AllocaIP,
/*Dependencies=*/{}, Info.HasNoWait));
} else {
Function *BeginMapperFunc = getOrCreateRuntimeFunctionPtr(
omp::OMPRTL___tgt_target_data_begin_mapper);
Expand Down
32 changes: 13 additions & 19 deletions llvm/lib/Transforms/IPO/OpenMPOpt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1178,15 +1178,12 @@ struct OpenMPOpt {

OpenMPIRBuilder::LocationDescription Loc(
InsertPointTy(ParentBB, ParentBB->end()), DL);
OpenMPIRBuilder::InsertPointOrErrorTy SeqAfterIP =
OMPInfoCache.OMPBuilder.createMaster(Loc, BodyGenCB, FiniCB);
assert(SeqAfterIP && "Unexpected error creating master");
OpenMPIRBuilder::InsertPointTy SeqAfterIP = cantFail(
OMPInfoCache.OMPBuilder.createMaster(Loc, BodyGenCB, FiniCB));
cantFail(
OMPInfoCache.OMPBuilder.createBarrier(SeqAfterIP, OMPD_parallel));

OpenMPIRBuilder::InsertPointOrErrorTy BarrierAfterIP =
OMPInfoCache.OMPBuilder.createBarrier(*SeqAfterIP, OMPD_parallel);
assert(BarrierAfterIP && "Unexpected error creating barrier");

BranchInst::Create(SeqAfterBB, SeqAfterIP->getBlock());
BranchInst::Create(SeqAfterBB, SeqAfterIP.getBlock());

LLVM_DEBUG(dbgs() << TAG << "After sequential inlining " << *OuterFn
<< "\n");
Expand Down Expand Up @@ -1256,12 +1253,11 @@ struct OpenMPOpt {
OriginalFn->getEntryBlock().getFirstInsertionPt());
// Create the merged parallel region with default proc binding, to
// avoid overriding binding settings, and without explicit cancellation.
OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPInfoCache.OMPBuilder.createParallel(
OpenMPIRBuilder::InsertPointTy AfterIP =
cantFail(OMPInfoCache.OMPBuilder.createParallel(
Loc, AllocaIP, BodyGenCB, PrivCB, FiniCB, nullptr, nullptr,
OMP_PROC_BIND_default, /* IsCancellable */ false);
assert(AfterIP && "Unexpected error creating parallel");
BranchInst::Create(AfterBB, AfterIP->getBlock());
OMP_PROC_BIND_default, /* IsCancellable */ false));
BranchInst::Create(AfterBB, AfterIP.getBlock());

// Perform the actual outlining.
OMPInfoCache.OMPBuilder.finalize(OriginalFn);
Expand Down Expand Up @@ -1297,12 +1293,10 @@ struct OpenMPOpt {
if (CI != MergableCIs.back()) {
// TODO: Remove barrier if the merged parallel region includes the
// 'nowait' clause.
OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
OMPInfoCache.OMPBuilder.createBarrier(
InsertPointTy(NewCI->getParent(),
NewCI->getNextNode()->getIterator()),
OMPD_parallel);
assert(AfterIP && "Unexpected error creating barrier");
cantFail(OMPInfoCache.OMPBuilder.createBarrier(
InsertPointTy(NewCI->getParent(),
NewCI->getNextNode()->getIterator()),
OMPD_parallel));
}

CI->eraseFromParent();
Expand Down
Loading

0 comments on commit b79ed87

Please sign in to comment.