Skip to content

Commit 573f816

Browse files
skatrakskatrak
committed
[MLIR][OpenMP][OMPIRBuilder] Error propagation across callbacks
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'.
1 parent 08330db commit 573f816

File tree

8 files changed

+1357
-819
lines changed

8 files changed

+1357
-819
lines changed

‎clang/lib/CodeGen/CGOpenMPRuntime.cpp‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,6 +1192,7 @@ struct PushAndPopStackRAII {
11921192
CodeGenFunction::JumpDest Dest =
11931193
CGF.getOMPCancelDestination(OMPD_parallel);
11941194
CGF.EmitBranchThroughCleanup(Dest);
1195+
return llvm::Error::success();
11951196
};
11961197

11971198
// TODO: Remove this once we emit parallel regions through the
@@ -2331,8 +2332,10 @@ void CGOpenMPRuntime::emitBarrierCall(CodeGenFunction &CGF, SourceLocation Loc,
23312332
auto *OMPRegionInfo =
23322333
dyn_cast_or_null<CGOpenMPRegionInfo>(CGF.CapturedStmtInfo);
23332334
if (CGF.CGM.getLangOpts().OpenMPIRBuilder) {
2334-
CGF.Builder.restoreIP(OMPBuilder.createBarrier(
2335-
CGF.Builder, Kind, ForceSimpleCall, EmitChecks));
2335+
auto Result = OMPBuilder.createBarrier(CGF.Builder, Kind, ForceSimpleCall,
2336+
EmitChecks);
2337+
assert(Result && "unexpected error creating barrier");
2338+
CGF.Builder.restoreIP(*Result);
23362339
return;
23372340
}
23382341

@@ -5928,8 +5931,10 @@ void CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(
59285931
return CGF.GenerateOpenMPCapturedStmtFunction(CS, D.getBeginLoc());
59295932
};
59305933

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

59345939
if (!OutlinedFn)
59355940
return;
@@ -9671,9 +9676,11 @@ static void emitTargetCallKernelLaunch(
96719676
NumTargetItems, RTArgs, NumIterations, NumTeams, NumThreads,
96729677
DynCGGroupMem, HasNoWait);
96739678

9674-
CGF.Builder.restoreIP(OMPRuntime->getOMPBuilder().emitKernelLaunch(
9679+
auto Result = OMPRuntime->getOMPBuilder().emitKernelLaunch(
96759680
CGF.Builder, OutlinedFn, OutlinedFnID, EmitTargetCallFallbackCB, Args,
9676-
DeviceID, RTLoc, AllocaIP));
9681+
DeviceID, RTLoc, AllocaIP);
9682+
assert(Result && "unexpected error creating kernel launch");
9683+
CGF.Builder.restoreIP(*Result);
96779684
};
96789685

96799686
if (RequiresOuterTask)
@@ -10350,9 +10357,11 @@ void CGOpenMPRuntime::emitTargetDataCalls(
1035010357
InsertPointTy CodeGenIP(CGF.Builder.GetInsertBlock(),
1035110358
CGF.Builder.GetInsertPoint());
1035210359
llvm::OpenMPIRBuilder::LocationDescription OmpLoc(CodeGenIP);
10353-
CGF.Builder.restoreIP(OMPBuilder.createTargetData(
10360+
auto Result = OMPBuilder.createTargetData(
1035410361
OmpLoc, AllocaIP, CodeGenIP, DeviceID, IfCondVal, Info, GenMapInfoCB,
10355-
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc));
10362+
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc);
10363+
assert(Result && "unexpected error creating target data");
10364+
CGF.Builder.restoreIP(*Result);
1035610365
}
1035710366

1035810367
void CGOpenMPRuntime::emitTargetDataStandAloneCall(

‎clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp‎

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

1756-
CGF.Builder.restoreIP(OMPBuilder.createReductionsGPU(
1756+
auto Result = OMPBuilder.createReductionsGPU(
17571757
OmpLoc, AllocaIP, CodeGenIP, ReductionInfos, false, TeamsReduction,
17581758
DistributeReduction, llvm::OpenMPIRBuilder::ReductionGenCBKind::Clang,
17591759
CGF.getTarget().getGridValue(), C.getLangOpts().OpenMPCUDAReductionBufNum,
1760-
RTLoc));
1760+
RTLoc);
1761+
assert(Result && "unexpected error creating GPU reductions");
1762+
CGF.Builder.restoreIP(*Result);
17611763
return;
17621764
}
17631765

‎clang/lib/CodeGen/CGStmtOpenMP.cpp‎

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1809,6 +1809,7 @@ void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
18091809
// thus calls destructors etc.
18101810
auto FiniCB = [this](InsertPointTy IP) {
18111811
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
1812+
return llvm::Error::success();
18121813
};
18131814

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

18361838
CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
18371839
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
18381840
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
18391841
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
1840-
Builder.restoreIP(
1842+
auto Result =
18411843
OMPBuilder.createParallel(Builder, AllocaIP, BodyGenCB, PrivCB, FiniCB,
1842-
IfCond, NumThreads, ProcBind, S.hasCancel()));
1844+
IfCond, NumThreads, ProcBind, S.hasCancel());
1845+
assert(Result && "unexpected error creating parallel");
1846+
Builder.restoreIP(*Result);
18431847
return;
18441848
}
18451849

@@ -2128,9 +2132,12 @@ void CodeGenFunction::EmitOMPCanonicalLoop(const OMPCanonicalLoop *S) {
21282132

21292133
RunCleanupsScope BodyScope(*this);
21302134
EmitStmt(BodyStmt);
2135+
return llvm::Error::success();
21312136
};
2132-
llvm::CanonicalLoopInfo *CL =
2133-
OMPBuilder.createCanonicalLoop(Builder, BodyGen, DistVal);
2137+
2138+
auto Result = OMPBuilder.createCanonicalLoop(Builder, BodyGen, DistVal);
2139+
assert(Result && "unexpected error creating parallel");
2140+
llvm::CanonicalLoopInfo *CL = *Result;
21342141

21352142
// Finish up the loop.
21362143
Builder.restoreIP(CL->getAfterIP());
@@ -4016,11 +4023,12 @@ static void emitOMPForDirective(const OMPLoopDirective &S, CodeGenFunction &CGF,
40164023
CGM.getOpenMPRuntime().getOMPBuilder();
40174024
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
40184025
CGF.AllocaInsertPt->getParent(), CGF.AllocaInsertPt->getIterator());
4019-
OMPBuilder.applyWorkshareLoop(
4026+
auto Result = OMPBuilder.applyWorkshareLoop(
40204027
CGF.Builder.getCurrentDebugLocation(), CLI, AllocaIP, NeedsBarrier,
40214028
SchedKind, ChunkSize, /*HasSimdModifier=*/false,
40224029
/*HasMonotonicModifier=*/false, /*HasNonmonotonicModifier=*/false,
40234030
/*HasOrderedClause=*/false);
4031+
assert(Result && "unexpected error creating workshare loop");
40244032
return;
40254033
}
40264034

@@ -4257,6 +4265,7 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
42574265

42584266
auto FiniCB = [this](InsertPointTy IP) {
42594267
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
4268+
return llvm::Error::success();
42604269
};
42614270

42624271
const CapturedStmt *ICS = S.getInnermostCapturedStmt();
@@ -4269,6 +4278,7 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
42694278
InsertPointTy CodeGenIP) {
42704279
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
42714280
*this, SubStmt, AllocaIP, CodeGenIP, "section");
4281+
return llvm::Error::success();
42724282
};
42734283
SectionCBVector.push_back(SectionCB);
42744284
}
@@ -4277,6 +4287,7 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
42774287
InsertPointTy CodeGenIP) {
42784288
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
42794289
*this, CapturedStmt, AllocaIP, CodeGenIP, "section");
4290+
return llvm::Error::success();
42804291
};
42814292
SectionCBVector.push_back(SectionCB);
42824293
}
@@ -4298,9 +4309,11 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
42984309
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
42994310
llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
43004311
AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
4301-
Builder.restoreIP(OMPBuilder.createSections(
4312+
auto Result = OMPBuilder.createSections(
43024313
Builder, AllocaIP, SectionCBVector, PrivCB, FiniCB, S.hasCancel(),
4303-
S.getSingleClause<OMPNowaitClause>()));
4314+
S.getSingleClause<OMPNowaitClause>());
4315+
assert(Result && "unexpected error creating sections");
4316+
Builder.restoreIP(*Result);
43044317
return;
43054318
}
43064319
{
@@ -4326,17 +4339,21 @@ void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {
43264339
const Stmt *SectionRegionBodyStmt = S.getAssociatedStmt();
43274340
auto FiniCB = [this](InsertPointTy IP) {
43284341
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
4342+
return llvm::Error::success();
43294343
};
43304344

43314345
auto BodyGenCB = [SectionRegionBodyStmt, this](InsertPointTy AllocaIP,
43324346
InsertPointTy CodeGenIP) {
43334347
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
43344348
*this, SectionRegionBodyStmt, AllocaIP, CodeGenIP, "section");
4349+
return llvm::Error::success();
43354350
};
43364351

43374352
LexicalScope Scope(*this, S.getSourceRange());
43384353
EmitStopPoint(&S);
4339-
Builder.restoreIP(OMPBuilder.createSection(Builder, BodyGenCB, FiniCB));
4354+
auto Result = OMPBuilder.createSection(Builder, BodyGenCB, FiniCB);
4355+
assert(Result && "unexpected error creating section");
4356+
Builder.restoreIP(*Result);
43404357

43414358
return;
43424359
}
@@ -4407,17 +4424,21 @@ void CodeGenFunction::EmitOMPMasterDirective(const OMPMasterDirective &S) {
44074424

44084425
auto FiniCB = [this](InsertPointTy IP) {
44094426
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
4427+
return llvm::Error::success();
44104428
};
44114429

44124430
auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
44134431
InsertPointTy CodeGenIP) {
44144432
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
44154433
*this, MasterRegionBodyStmt, AllocaIP, CodeGenIP, "master");
4434+
return llvm::Error::success();
44164435
};
44174436

44184437
LexicalScope Scope(*this, S.getSourceRange());
44194438
EmitStopPoint(&S);
4420-
Builder.restoreIP(OMPBuilder.createMaster(Builder, BodyGenCB, FiniCB));
4439+
auto Result = OMPBuilder.createMaster(Builder, BodyGenCB, FiniCB);
4440+
assert(Result && "unexpected error creating master");
4441+
Builder.restoreIP(*Result);
44214442

44224443
return;
44234444
}
@@ -4453,18 +4474,22 @@ void CodeGenFunction::EmitOMPMaskedDirective(const OMPMaskedDirective &S) {
44534474

44544475
auto FiniCB = [this](InsertPointTy IP) {
44554476
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
4477+
return llvm::Error::success();
44564478
};
44574479

44584480
auto BodyGenCB = [MaskedRegionBodyStmt, this](InsertPointTy AllocaIP,
44594481
InsertPointTy CodeGenIP) {
44604482
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
44614483
*this, MaskedRegionBodyStmt, AllocaIP, CodeGenIP, "masked");
4484+
return llvm::Error::success();
44624485
};
44634486

44644487
LexicalScope Scope(*this, S.getSourceRange());
44654488
EmitStopPoint(&S);
4466-
Builder.restoreIP(
4467-
OMPBuilder.createMasked(Builder, BodyGenCB, FiniCB, FilterVal));
4489+
auto Result =
4490+
OMPBuilder.createMasked(Builder, BodyGenCB, FiniCB, FilterVal);
4491+
assert(Result && "unexpected error creating masked");
4492+
Builder.restoreIP(*Result);
44684493

44694494
return;
44704495
}
@@ -4493,19 +4518,23 @@ void CodeGenFunction::EmitOMPCriticalDirective(const OMPCriticalDirective &S) {
44934518

44944519
auto FiniCB = [this](InsertPointTy IP) {
44954520
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
4521+
return llvm::Error::success();
44964522
};
44974523

44984524
auto BodyGenCB = [CriticalRegionBodyStmt, this](InsertPointTy AllocaIP,
44994525
InsertPointTy CodeGenIP) {
45004526
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
45014527
*this, CriticalRegionBodyStmt, AllocaIP, CodeGenIP, "critical");
4528+
return llvm::Error::success();
45024529
};
45034530

45044531
LexicalScope Scope(*this, S.getSourceRange());
45054532
EmitStopPoint(&S);
4506-
Builder.restoreIP(OMPBuilder.createCritical(
4507-
Builder, BodyGenCB, FiniCB, S.getDirectiveName().getAsString(),
4508-
HintInst));
4533+
auto Result =
4534+
OMPBuilder.createCritical(Builder, BodyGenCB, FiniCB,
4535+
S.getDirectiveName().getAsString(), HintInst);
4536+
assert(Result && "unexpected error creating critical");
4537+
Builder.restoreIP(*Result);
45094538

45104539
return;
45114540
}
@@ -5464,11 +5493,14 @@ void CodeGenFunction::EmitOMPTaskgroupDirective(
54645493
InsertPointTy CodeGenIP) {
54655494
Builder.restoreIP(CodeGenIP);
54665495
EmitStmt(S.getInnermostCapturedStmt()->getCapturedStmt());
5496+
return llvm::Error::success();
54675497
};
54685498
CodeGenFunction::CGCapturedStmtInfo CapStmtInfo;
54695499
if (!CapturedStmtInfo)
54705500
CapturedStmtInfo = &CapStmtInfo;
5471-
Builder.restoreIP(OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB));
5501+
auto Result = OMPBuilder.createTaskgroup(Builder, AllocaIP, BodyGenCB);
5502+
assert(Result && "unexpected error creating taskgroup");
5503+
Builder.restoreIP(*Result);
54725504
return;
54735505
}
54745506
auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
@@ -6041,6 +6073,7 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
60416073

60426074
auto FiniCB = [this](InsertPointTy IP) {
60436075
OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
6076+
return llvm::Error::success();
60446077
};
60456078

60466079
auto BodyGenCB = [&S, C, this](InsertPointTy AllocaIP,
@@ -6064,11 +6097,14 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
60646097
OMPBuilderCBHelpers::EmitOMPInlinedRegionBody(
60656098
*this, CS->getCapturedStmt(), AllocaIP, CodeGenIP, "ordered");
60666099
}
6100+
return llvm::Error::success();
60676101
};
60686102

60696103
OMPLexicalScope Scope(*this, S, OMPD_unknown);
6070-
Builder.restoreIP(
6071-
OMPBuilder.createOrderedThreadsSimd(Builder, BodyGenCB, FiniCB, !C));
6104+
auto Result =
6105+
OMPBuilder.createOrderedThreadsSimd(Builder, BodyGenCB, FiniCB, !C);
6106+
assert(Result && "unexpected error creating ordered");
6107+
Builder.restoreIP(*Result);
60726108
}
60736109
return;
60746110
}
@@ -7344,8 +7380,10 @@ void CodeGenFunction::EmitOMPCancelDirective(const OMPCancelDirective &S) {
73447380
if (IfCond)
73457381
IfCondition = EmitScalarExpr(IfCond,
73467382
/*IgnoreResultAssign=*/true);
7347-
return Builder.restoreIP(
7348-
OMPBuilder.createCancel(Builder, IfCondition, S.getCancelRegion()));
7383+
auto Result =
7384+
OMPBuilder.createCancel(Builder, IfCondition, S.getCancelRegion());
7385+
assert(Result && "unexpected error creating cancel");
7386+
return Builder.restoreIP(*Result);
73497387
}
73507388
}
73517389

0 commit comments

Comments
 (0)