Skip to content

Commit 23af823

Browse files
skatrakskatrak
committed
[OpenMP][OMPIRBuilder] Handle non-failing calls properly
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.
1 parent 03e7862 commit 23af823

File tree

6 files changed

+524
-499
lines changed

6 files changed

+524
-499
lines changed

‎clang/lib/CodeGen/CGOpenMPRuntime.cpp‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,11 +2327,10 @@ void CGOpenMPRuntime::emitBarrierCall(CodeGenFunction &CGF, SourceLocation Loc,
23272327
auto *OMPRegionInfo =
23282328
dyn_cast_or_null<CGOpenMPRegionInfo>(CGF.CapturedStmtInfo);
23292329
if (CGF.CGM.getLangOpts().OpenMPIRBuilder) {
2330-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
2331-
OMPBuilder.createBarrier(CGF.Builder, Kind, ForceSimpleCall,
2332-
EmitChecks);
2333-
assert(AfterIP && "unexpected error creating barrier");
2334-
CGF.Builder.restoreIP(*AfterIP);
2330+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
2331+
cantFail(OMPBuilder.createBarrier(CGF.Builder, Kind, ForceSimpleCall,
2332+
EmitChecks));
2333+
CGF.Builder.restoreIP(AfterIP);
23352334
return;
23362335
}
23372336

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

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

59415939
if (!OutlinedFn)
59425940
return;
@@ -9409,12 +9407,11 @@ static void emitTargetCallKernelLaunch(
94099407
NumTargetItems, RTArgs, NumIterations, NumTeams, NumThreads,
94109408
DynCGGroupMem, HasNoWait);
94119409

9412-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
9413-
OMPRuntime->getOMPBuilder().emitKernelLaunch(
9410+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
9411+
cantFail(OMPRuntime->getOMPBuilder().emitKernelLaunch(
94149412
CGF.Builder, OutlinedFnID, EmitTargetCallFallbackCB, Args, DeviceID,
9415-
RTLoc, AllocaIP);
9416-
assert(AfterIP && "unexpected error creating kernel launch");
9417-
CGF.Builder.restoreIP(*AfterIP);
9413+
RTLoc, AllocaIP));
9414+
CGF.Builder.restoreIP(AfterIP);
94189415
};
94199416

94209417
if (RequiresOuterTask)
@@ -10091,12 +10088,11 @@ void CGOpenMPRuntime::emitTargetDataCalls(
1009110088
InsertPointTy CodeGenIP(CGF.Builder.GetInsertBlock(),
1009210089
CGF.Builder.GetInsertPoint());
1009310090
llvm::OpenMPIRBuilder::LocationDescription OmpLoc(CodeGenIP);
10094-
llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP =
10095-
OMPBuilder.createTargetData(
10091+
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
10092+
cantFail(OMPBuilder.createTargetData(
1009610093
OmpLoc, AllocaIP, CodeGenIP, DeviceID, IfCondVal, Info, GenMapInfoCB,
10097-
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc);
10098-
assert(AfterIP && "unexpected error creating target data");
10099-
CGF.Builder.restoreIP(*AfterIP);
10094+
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc));
10095+
CGF.Builder.restoreIP(AfterIP);
1010010096
}
1010110097

1010210098
void CGOpenMPRuntime::emitTargetDataStandAloneCall(

‎clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp‎

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

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

‎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
@@ -4243,23 +4243,19 @@ OpenMPIRBuilder::applyStaticChunkedWorkshareLoop(DebugLoc DL,
42434243
// Create outer "dispatch" loop for enumerating the chunks.
42444244
BasicBlock *DispatchEnter = splitBB(Builder, true);
42454245
Value *DispatchCounter;
4246-
Expected<CanonicalLoopInfo *> LoopResult = createCanonicalLoop(
4246+
4247+
// It is safe to assume this didn't return an error because the callback
4248+
// passed into createCanonicalLoop is the only possible error source, and it
4249+
// always returns success.
4250+
CanonicalLoopInfo *DispatchCLI = cantFail(createCanonicalLoop(
42474251
{Builder.saveIP(), DL},
42484252
[&](InsertPointTy BodyIP, Value *Counter) {
42494253
DispatchCounter = Counter;
42504254
return Error::success();
42514255
},
42524256
FirstChunkStart, CastedTripCount, NextChunkStride,
42534257
/*IsSigned=*/false, /*InclusiveStop=*/false, /*ComputeIP=*/{},
4254-
"dispatch");
4255-
if (!LoopResult) {
4256-
// It is safe to assume this didn't return an error because the callback
4257-
// passed into createCanonicalLoop is the only possible error source, and it
4258-
// always returns success. Need to still cast the result into bool to avoid
4259-
// runtime errors.
4260-
llvm_unreachable("unexpected error creating canonical loop");
4261-
}
4262-
CanonicalLoopInfo *DispatchCLI = *LoopResult;
4258+
"dispatch"));
42634259

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

65636559
bool RequiresOuterTargetTask = Info.HasNoWait;
6564-
if (!RequiresOuterTargetTask) {
6565-
Error Err = TaskBodyCB(/*DeviceID=*/nullptr, /*RTLoc=*/nullptr,
6566-
/*TargetTaskAllocaIP=*/{});
6567-
assert(!Err && "TaskBodyCB expected to succeed");
6568-
} else {
6569-
InsertPointOrErrorTy AfterIP =
6570-
emitTargetTask(TaskBodyCB, DeviceID, SrcLocInfo, AllocaIP,
6571-
/*Dependencies=*/{}, Info.HasNoWait);
6572-
assert(AfterIP && "TaskBodyCB expected to succeed");
6573-
}
6560+
if (!RequiresOuterTargetTask)
6561+
cantFail(TaskBodyCB(/*DeviceID=*/nullptr, /*RTLoc=*/nullptr,
6562+
/*TargetTaskAllocaIP=*/{}));
6563+
else
6564+
cantFail(emitTargetTask(TaskBodyCB, DeviceID, SrcLocInfo, AllocaIP,
6565+
/*Dependencies=*/{}, Info.HasNoWait));
65746566
} else {
65756567
Function *BeginMapperFunc = getOrCreateRuntimeFunctionPtr(
65766568
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)