Skip to content

Commit 7584acb

Browse files
skatrakskatrak
committed
[OpenMP][OMPIRBuilder] Handle non-failing calls properly
The macro 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. 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.
1 parent 3ce0dbb commit 7584acb

File tree

6 files changed

+353
-475
lines changed

6 files changed

+353
-475
lines changed

‎clang/lib/CodeGen/CGOpenMPRuntime.cpp

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,11 +2332,10 @@ void CGOpenMPRuntime::emitBarrierCall(CodeGenFunction &CGF, SourceLocation Loc,
23322332
auto *OMPRegionInfo =
23332333
dyn_cast_or_null<CGOpenMPRegionInfo>(CGF.CapturedStmtInfo);
23342334
if (CGF.CGM.getLangOpts().OpenMPIRBuilder) {
2335-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
2336-
OMPBuilder.createBarrier(CGF.Builder, Kind, ForceSimpleCall,
2337-
EmitChecks);
2338-
assert(AfterIP && "unexpected error creating barrier");
2339-
CGF.Builder.restoreIP(*AfterIP);
2335+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
2336+
cantFail(OMPBuilder.createBarrier(CGF.Builder, Kind, ForceSimpleCall,
2337+
EmitChecks));
2338+
CGF.Builder.restoreIP(AfterIP);
23402339
return;
23412340
}
23422341

@@ -5932,10 +5931,9 @@ void CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(
59325931
return CGF.GenerateOpenMPCapturedStmtFunction(CS, D.getBeginLoc());
59335932
};
59345933

5935-
llvm::Error Err = OMPBuilder.emitTargetRegionFunction(
5934+
cantFail(OMPBuilder.emitTargetRegionFunction(
59365935
EntryInfo, GenerateOutlinedFunction, IsOffloadEntry, OutlinedFn,
5937-
OutlinedFnID);
5938-
assert(!Err && "unexpected error creating target region");
5936+
OutlinedFnID));
59395937

59405938
if (!OutlinedFn)
59415939
return;
@@ -9676,12 +9674,11 @@ static void emitTargetCallKernelLaunch(
96769674
NumTargetItems, RTArgs, NumIterations, NumTeams, NumThreads,
96779675
DynCGGroupMem, HasNoWait);
96789676

9679-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
9680-
OMPRuntime->getOMPBuilder().emitKernelLaunch(
9677+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
9678+
cantFail(OMPRuntime->getOMPBuilder().emitKernelLaunch(
96819679
CGF.Builder, OutlinedFnID, EmitTargetCallFallbackCB, Args, DeviceID,
9682-
RTLoc, AllocaIP);
9683-
assert(AfterIP && "unexpected error creating kernel launch");
9684-
CGF.Builder.restoreIP(*AfterIP);
9680+
RTLoc, AllocaIP));
9681+
CGF.Builder.restoreIP(AfterIP);
96859682
};
96869683

96879684
if (RequiresOuterTask)
@@ -10358,12 +10355,11 @@ void CGOpenMPRuntime::emitTargetDataCalls(
1035810355
InsertPointTy CodeGenIP(CGF.Builder.GetInsertBlock(),
1035910356
CGF.Builder.GetInsertPoint());
1036010357
llvm::OpenMPIRBuilder::LocationDescription OmpLoc(CodeGenIP);
10361-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
10362-
OMPBuilder.createTargetData(
10358+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
10359+
cantFail(OMPBuilder.createTargetData(
1036310360
OmpLoc, AllocaIP, CodeGenIP, DeviceID, IfCondVal, Info, GenMapInfoCB,
10364-
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc);
10365-
assert(AfterIP && "unexpected error creating target data");
10366-
CGF.Builder.restoreIP(*AfterIP);
10361+
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc));
10362+
CGF.Builder.restoreIP(AfterIP);
1036710363
}
1036810364

1036910365
void CGOpenMPRuntime::emitTargetDataStandAloneCall(

‎clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1753,14 +1753,13 @@ void CGOpenMPRuntimeGPU::emitReduction(
17531753
Idx++;
17541754
}
17551755

1756-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
1757-
OMPBuilder.createReductionsGPU(
1756+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
1757+
cantFail(OMPBuilder.createReductionsGPU(
17581758
OmpLoc, AllocaIP, CodeGenIP, ReductionInfos, false, TeamsReduction,
17591759
DistributeReduction, llvm::OpenMPIRBuilder::ReductionGenCBKind::Clang,
17601760
CGF.getTarget().getGridValue(),
1761-
C.getLangOpts().OpenMPCUDAReductionBufNum, RTLoc);
1762-
assert(AfterIP && "unexpected error creating GPU reductions");
1763-
CGF.Builder.restoreIP(*AfterIP);
1761+
C.getLangOpts().OpenMPCUDAReductionBufNum, RTLoc));
1762+
CGF.Builder.restoreIP(AfterIP);
17641763
return;
17651764
}
17661765

‎clang/lib/CodeGen/CGStmtOpenMP.cpp

Lines changed: 38 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1839,11 +1839,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
18391839
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
18401840
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
18411841
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
1842-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
1842+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP = cantFail(
18431843
OMPBuilder.createParallel(Builder, AllocaIP, BodyGenCB, PrivCB, FiniCB,
1844-
IfCond, NumThreads, ProcBind, S.hasCancel());
1845-
assert(AfterIP && "unexpected error creating parallel");
1846-
Builder.restoreIP(*AfterIP);
1844+
IfCond, NumThreads, ProcBind, S.hasCancel()));
1845+
Builder.restoreIP(AfterIP);
18471846
return;
18481847
}
18491848

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

2138-
llvm::Expected<llvm::CanonicalLoopInfo *> Result =
2139-
OMPBuilder.createCanonicalLoop(Builder, BodyGen, DistVal);
2140-
assert(Result && "unexpected error creating canonical loop");
2141-
llvm::CanonicalLoopInfo *CL = *Result;
2137+
llvm::CanonicalLoopInfo *CL =
2138+
cantFail(OMPBuilder.createCanonicalLoop(Builder, BodyGen, DistVal));
21422139

21432140
// Finish up the loop.
21442141
Builder.restoreIP(CL->getAfterIP());
@@ -4024,13 +4021,11 @@ static void emitOMPForDirective(const OMPLoopDirective &S, CodeGenFunction &CGF,
40244021
CGM.getOpenMPRuntime().getOMPBuilder();
40254022
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
40264023
CGF.AllocaInsertPt->getParent(), CGF.AllocaInsertPt->getIterator());
4027-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
4028-
OMPBuilder.applyWorkshareLoop(
4029-
CGF.Builder.getCurrentDebugLocation(), CLI, AllocaIP,
4030-
NeedsBarrier, SchedKind, ChunkSize, /*HasSimdModifier=*/false,
4031-
/*HasMonotonicModifier=*/false, /*HasNonmonotonicModifier=*/false,
4032-
/*HasOrderedClause=*/false);
4033-
assert(AfterIP && "unexpected error creating workshare loop");
4024+
cantFail(OMPBuilder.applyWorkshareLoop(
4025+
CGF.Builder.getCurrentDebugLocation(), CLI, AllocaIP, NeedsBarrier,
4026+
SchedKind, ChunkSize, /*HasSimdModifier=*/false,
4027+
/*HasMonotonicModifier=*/false, /*HasNonmonotonicModifier=*/false,
4028+
/*HasOrderedClause=*/false));
40344029
return;
40354030
}
40364031

@@ -4311,12 +4306,11 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
43114306
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
43124307
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
43134308
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
4314-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
4315-
OMPBuilder.createSections(Builder, AllocaIP, SectionCBVector, PrivCB,
4316-
FiniCB, S.hasCancel(),
4317-
S.getSingleClause<OMPNowaitClause>());
4318-
assert(AfterIP && "unexpected error creating sections");
4319-
Builder.restoreIP(*AfterIP);
4309+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
4310+
cantFail(OMPBuilder.createSections(
4311+
Builder, AllocaIP, SectionCBVector, PrivCB, FiniCB, S.hasCancel(),
4312+
S.getSingleClause<OMPNowaitClause>()));
4313+
Builder.restoreIP(AfterIP);
43204314
return;
43214315
}
43224316
{
@@ -4354,10 +4348,9 @@ void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {
43544348

43554349
LexicalScope Scope(*this, S.getSourceRange());
43564350
EmitStopPoint(&S);
4357-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
4358-
OMPBuilder.createSection(Builder, BodyGenCB, FiniCB);
4359-
assert(AfterIP && "unexpected error creating section");
4360-
Builder.restoreIP(*AfterIP);
4351+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
4352+
cantFail(OMPBuilder.createSection(Builder, BodyGenCB, FiniCB));
4353+
Builder.restoreIP(AfterIP);
43614354

43624355
return;
43634356
}
@@ -4440,10 +4433,9 @@ void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {
44404433

44414434
LexicalScope Scope(*this, S.getSourceRange());
44424435
EmitStopPoint(&S);
4443-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
4444-
OMPBuilder.createMaster(Builder, BodyGenCB, FiniCB);
4445-
assert(AfterIP && "unexpected error creating master");
4446-
Builder.restoreIP(*AfterIP);
4436+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
4437+
cantFail(OMPBuilder.createMaster(Builder, BodyGenCB, FiniCB));
4438+
Builder.restoreIP(AfterIP);
44474439

44484440
return;
44494441
}
@@ -4491,10 +4483,9 @@ void CodeGenFunction::EmitOMPMaskedDirective(const OMPMaskedDirective &S) {
44914483

44924484
LexicalScope Scope(*this, S.getSourceRange());
44934485
EmitStopPoint(&S);
4494-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
4495-
OMPBuilder.createMasked(Builder, BodyGenCB, FiniCB, FilterVal);
4496-
assert(AfterIP && "unexpected error creating masked");
4497-
Builder.restoreIP(*AfterIP);
4486+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP = cantFail(
4487+
OMPBuilder.createMasked(Builder, BodyGenCB, FiniCB, FilterVal));
4488+
Builder.restoreIP(AfterIP);
44984489

44994490
return;
45004491
}
@@ -4535,11 +4526,11 @@ void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {
45354526

45364527
LexicalScope Scope(*this, S.getSourceRange());
45374528
EmitStopPoint(&S);
4538-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
4539-
OMPBuilder.createCritical(Builder, BodyGenCB, FiniCB,
4540-
S.getDirectiveName().getAsString(), HintInst);
4541-
assert(AfterIP && "unexpected error creating critical");
4542-
Builder.restoreIP(*AfterIP);
4529+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
4530+
cantFail(OMPBuilder.createCritical(Builder, BodyGenCB, FiniCB,
4531+
S.getDirectiveName().getAsString(),
4532+
HintInst));
4533+
Builder.restoreIP(AfterIP);
45434534

45444535
return;
45454536
}
@@ -5503,10 +5494,9 @@ void CodeGenFunction::EmitOMPTaskgroupDirective(
55035494
CodeGenFunction::CGCapturedStmtInfo CapStmtInfo;
55045495
if (!CapturedStmtInfo)
55055496
CapturedStmtInfo = &CapStmtInfo;
5506-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
5507-
OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB);
5508-
assert(AfterIP && "unexpected error creating taskgroup");
5509-
Builder.restoreIP(*AfterIP);
5497+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
5498+
cantFail(OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB));
5499+
Builder.restoreIP(AfterIP);
55105500
return;
55115501
}
55125502
auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
@@ -6109,10 +6099,9 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
61096099
};
61106100

61116101
OMPLexicalScope Scope(*this, S, OMPD_unknown);
6112-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
6113-
OMPBuilder.createOrderedThreadsSimd(Builder, BodyGenCB, FiniCB, !C);
6114-
assert(AfterIP && "unexpected error creating ordered");
6115-
Builder.restoreIP(*AfterIP);
6102+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP = cantFail(
6103+
OMPBuilder.createOrderedThreadsSimd(Builder, BodyGenCB, FiniCB, !C));
6104+
Builder.restoreIP(AfterIP);
61166105
}
61176106
return;
61186107
}
@@ -7388,10 +7377,9 @@ void CodeGenFunction::EmitOMPCancelDirective(const OMPCancelDirective &S) {
73887377
if (IfCond)
73897378
IfCondition = EmitScalarExpr(IfCond,
73907379
/*IgnoreResultAssign=*/true);
7391-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
7392-
OMPBuilder.createCancel(Builder, IfCondition, S.getCancelRegion());
7393-
assert(AfterIP && "unexpected error creating cancel");
7394-
return Builder.restoreIP(*AfterIP);
7380+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP = cantFail(
7381+
OMPBuilder.createCancel(Builder, IfCondition, S.getCancelRegion()));
7382+
return Builder.restoreIP(AfterIP);
73957383
}
73967384
}
73977385

‎llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4225,23 +4225,19 @@ OpenMPIRBuilder::applyStaticChunkedWorkshareLoop(DebugLoc DL,
42254225
// Create outer "dispatch" loop for enumerating the chunks.
42264226
BasicBlock *DispatchEnter = splitBB(Builder, true);
42274227
Value *DispatchCounter;
4228-
Expected<CanonicalLoopInfo *> LoopResult = createCanonicalLoop(
4228+
4229+
// It is safe to assume this didn't return an error because the callback
4230+
// passed into createCanonicalLoop is the only possible error source, and it
4231+
// always returns success.
4232+
CanonicalLoopInfo *DispatchCLI = cantFail(createCanonicalLoop(
42294233
{Builder.saveIP(), DL},
42304234
[&](InsertPointTy BodyIP, Value *Counter) {
42314235
DispatchCounter = Counter;
42324236
return Error::success();
42334237
},
42344238
FirstChunkStart, CastedTripCount, NextChunkStride,
42354239
/*IsSigned=*/false, /*InclusiveStop=*/false, /*ComputeIP=*/{},
4236-
"dispatch");
4237-
if (!LoopResult) {
4238-
// It is safe to assume this didn't return an error because the callback
4239-
// passed into createCanonicalLoop is the only possible error source, and it
4240-
// always returns success. Need to still cast the result into bool to avoid
4241-
// runtime errors.
4242-
llvm_unreachable("unexpected error creating canonical loop");
4243-
}
4244-
CanonicalLoopInfo *DispatchCLI = *LoopResult;
4240+
"dispatch"));
42454241

42464242
// Remember the BasicBlocks of the dispatch loop we need, then invalidate to
42474243
// not have to preserve the canonical invariant.
@@ -6542,16 +6538,12 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTargetData(
65426538
};
65436539

65446540
bool RequiresOuterTargetTask = Info.HasNoWait;
6545-
if (!RequiresOuterTargetTask) {
6546-
Error Err = TaskBodyCB(/*DeviceID=*/nullptr, /*RTLoc=*/nullptr,
6547-
/*TargetTaskAllocaIP=*/{});
6548-
assert(!Err && "TaskBodyCB expected to succeed");
6549-
} else {
6550-
InsertPointOrErrorTy AfterIP =
6551-
emitTargetTask(TaskBodyCB, DeviceID, SrcLocInfo, AllocaIP,
6552-
/*Dependencies=*/{}, Info.HasNoWait);
6553-
assert(AfterIP && "TaskBodyCB expected to succeed");
6554-
}
6541+
if (!RequiresOuterTargetTask)
6542+
cantFail(TaskBodyCB(/*DeviceID=*/nullptr, /*RTLoc=*/nullptr,
6543+
/*TargetTaskAllocaIP=*/{}));
6544+
else
6545+
cantFail(emitTargetTask(TaskBodyCB, DeviceID, SrcLocInfo, AllocaIP,
6546+
/*Dependencies=*/{}, Info.HasNoWait));
65556547
} else {
65566548
Function *BeginMapperFunc = getOrCreateRuntimeFunctionPtr(
65576549
omp::OMPRTL___tgt_target_data_begin_mapper);

‎llvm/lib/Transforms/IPO/OpenMPOpt.cpp

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,15 +1178,12 @@ struct OpenMPOpt {
11781178

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

1185-
OpenMPIRBuilder::InsertPointOrErrorTy BarrierAfterIP =
1186-
OMPInfoCache.OMPBuilder.createBarrier(*SeqAfterIP, OMPD_parallel);
1187-
assert(BarrierAfterIP && "Unexpected error creating barrier");
1188-
1189-
BranchInst::Create(SeqAfterBB, SeqAfterIP->getBlock());
1186+
BranchInst::Create(SeqAfterBB, SeqAfterIP.getBlock());
11901187

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

12661262
// Perform the actual outlining.
12671263
OMPInfoCache.OMPBuilder.finalize(OriginalFn);
@@ -1297,12 +1293,10 @@ struct OpenMPOpt {
12971293
if (CI != MergableCIs.back()) {
12981294
// TODO: Remove barrier if the merged parallel region includes the
12991295
// 'nowait' clause.
1300-
OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
1301-
OMPInfoCache.OMPBuilder.createBarrier(
1302-
InsertPointTy(NewCI->getParent(),
1303-
NewCI->getNextNode()->getIterator()),
1304-
OMPD_parallel);
1305-
assert(AfterIP && "Unexpected error creating barrier");
1296+
cantFail(OMPInfoCache.OMPBuilder.createBarrier(
1297+
InsertPointTy(NewCI->getParent(),
1298+
NewCI->getNextNode()->getIterator()),
1299+
OMPD_parallel));
13061300
}
13071301

13081302
CI->eraseFromParent();

0 commit comments

Comments
 (0)