Skip to content

Commit 841c4dc

Browse files
skatrakskatrak
committed
[Flang][OpenMP][Lower] Fail compilation with TODO errors for unsupported clauses
This patch explicitly marks supported clauses for OpenMP directives missing an implementation, so that compilation fails if they are specified, rather than silently ignoring them. Differential Revision: https://reviews.llvm.org/D158076
1 parent 048b506 commit 841c4dc

File tree

3 files changed

+119
-39
lines changed

3 files changed

+119
-39
lines changed

‎flang/lib/Lower/OpenMP.cpp

Lines changed: 109 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "flang/Optimizer/Builder/FIRBuilder.h"
2222
#include "flang/Optimizer/Builder/Todo.h"
2323
#include "flang/Optimizer/HLFIR/HLFIROps.h"
24+
#include "flang/Parser/dump-parse-tree.h"
2425
#include "flang/Parser/parse-tree.h"
2526
#include "flang/Semantics/openmp-directive-sets.h"
2627
#include "flang/Semantics/tools.h"
@@ -549,15 +550,23 @@ class ClauseProcessor {
549550
llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
550551
&useDeviceSymbols) const;
551552

553+
// Call this method for these clauses that should be supported but are not
554+
// implemented yet. It triggers a compilation error if any of the given
555+
// clauses is found.
556+
template <typename... Ts>
557+
void processTODO(mlir::Location currentLocation,
558+
llvm::omp::Directive directive) const;
559+
552560
private:
553561
using ClauseIterator = std::list<ClauseTy>::const_iterator;
554562

555563
/// Utility to find a clause within a range in the clause list.
556564
template <typename T>
557565
static ClauseIterator findClause(ClauseIterator begin, ClauseIterator end) {
558-
for (ClauseIterator it = begin; it != end; ++it)
566+
for (ClauseIterator it = begin; it != end; ++it) {
559567
if (std::get_if<T>(&it->u))
560568
return it;
569+
}
561570

562571
return end;
563572
}
@@ -1773,6 +1782,24 @@ bool ClauseProcessor::processUseDevicePtr(
17731782
});
17741783
}
17751784

1785+
template <typename... Ts>
1786+
void ClauseProcessor::processTODO(mlir::Location currentLocation,
1787+
llvm::omp::Directive directive) const {
1788+
auto checkUnhandledClause = [&](const auto *x) {
1789+
if (!x)
1790+
return;
1791+
TODO(currentLocation,
1792+
"Unhandled clause " +
1793+
llvm::StringRef(Fortran::parser::ParseTreeDumper::GetNodeName(*x))
1794+
.upper() +
1795+
" in " + llvm::omp::getOpenMPDirectiveName(directive).upper() +
1796+
" construct");
1797+
};
1798+
1799+
for (ClauseIterator it = clauses.v.begin(); it != clauses.v.end(); ++it)
1800+
(checkUnhandledClause(std::get_if<Ts>(&it->u)), ...);
1801+
}
1802+
17761803
//===----------------------------------------------------------------------===//
17771804
// Code generation helper functions
17781805
//===----------------------------------------------------------------------===//
@@ -2213,8 +2240,11 @@ genSingleOp(Fortran::lower::AbstractConverter &converter,
22132240
llvm::SmallVector<mlir::Value> allocateOperands, allocatorOperands;
22142241
mlir::UnitAttr nowaitAttr;
22152242

2216-
ClauseProcessor(converter, beginClauseList)
2217-
.processAllocate(allocatorOperands, allocateOperands);
2243+
ClauseProcessor cp(converter, beginClauseList);
2244+
cp.processAllocate(allocatorOperands, allocateOperands);
2245+
cp.processTODO<Fortran::parser::OmpClause::Copyprivate>(
2246+
currentLocation, llvm::omp::Directive::OMPD_single);
2247+
22182248
ClauseProcessor(converter, endClauseList).processNowait(nowaitAttr);
22192249

22202250
return genOpWithBody<mlir::omp::SingleOp>(
@@ -2244,6 +2274,10 @@ genTaskOp(Fortran::lower::AbstractConverter &converter,
22442274
cp.processMergeable(mergeableAttr);
22452275
cp.processPriority(stmtCtx, priorityClauseOperand);
22462276
cp.processDepend(dependTypeOperands, dependOperands);
2277+
cp.processTODO<Fortran::parser::OmpClause::InReduction,
2278+
Fortran::parser::OmpClause::Detach,
2279+
Fortran::parser::OmpClause::Affinity>(
2280+
currentLocation, llvm::omp::Directive::OMPD_task);
22472281

22482282
return genOpWithBody<mlir::omp::TaskOp>(
22492283
converter, eval, currentLocation, /*outerCombined=*/false, &clauseList,
@@ -2263,9 +2297,10 @@ genTaskGroupOp(Fortran::lower::AbstractConverter &converter,
22632297
mlir::Location currentLocation,
22642298
const Fortran::parser::OmpClauseList &clauseList) {
22652299
llvm::SmallVector<mlir::Value> allocateOperands, allocatorOperands;
2266-
// TODO: Add task_reduction support
2267-
ClauseProcessor(converter, clauseList)
2268-
.processAllocate(allocatorOperands, allocateOperands);
2300+
ClauseProcessor cp(converter, clauseList);
2301+
cp.processAllocate(allocatorOperands, allocateOperands);
2302+
cp.processTODO<Fortran::parser::OmpClause::TaskReduction>(
2303+
currentLocation, llvm::omp::Directive::OMPD_taskgroup);
22692304
return genOpWithBody<mlir::omp::TaskGroupOp>(
22702305
converter, eval, currentLocation, /*outerCombined=*/false, &clauseList,
22712306
/*task_reduction_vars=*/mlir::ValueRange(),
@@ -2323,12 +2358,15 @@ genEnterExitDataOp(Fortran::lower::AbstractConverter &converter,
23232358
llvm::SmallVector<mlir::IntegerAttr> mapTypes;
23242359

23252360
Fortran::parser::OmpIfClause::DirectiveNameModifier directiveName;
2361+
llvm::omp::Directive directive;
23262362
if constexpr (std::is_same_v<OpTy, mlir::omp::EnterDataOp>) {
23272363
directiveName =
23282364
Fortran::parser::OmpIfClause::DirectiveNameModifier::TargetEnterData;
2365+
directive = llvm::omp::Directive::OMPD_target_enter_data;
23292366
} else if constexpr (std::is_same_v<OpTy, mlir::omp::ExitDataOp>) {
23302367
directiveName =
23312368
Fortran::parser::OmpIfClause::DirectiveNameModifier::TargetExitData;
2369+
directive = llvm::omp::Directive::OMPD_target_exit_data;
23322370
} else {
23332371
return nullptr;
23342372
}
@@ -2338,6 +2376,8 @@ genEnterExitDataOp(Fortran::lower::AbstractConverter &converter,
23382376
cp.processDevice(stmtCtx, deviceOperand);
23392377
cp.processNowait(nowaitAttr);
23402378
cp.processMap(mapOperands, mapTypes);
2379+
cp.processTODO<Fortran::parser::OmpClause::Depend>(currentLocation,
2380+
directive);
23412381

23422382
llvm::SmallVector<mlir::Attribute> mapTypesAttr(mapTypes.begin(),
23432383
mapTypes.end());
@@ -2370,6 +2410,17 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
23702410
cp.processThreadLimit(stmtCtx, threadLimitOperand);
23712411
cp.processNowait(nowaitAttr);
23722412
cp.processMap(mapOperands, mapTypes);
2413+
cp.processTODO<Fortran::parser::OmpClause::Private,
2414+
Fortran::parser::OmpClause::Depend,
2415+
Fortran::parser::OmpClause::Firstprivate,
2416+
Fortran::parser::OmpClause::IsDevicePtr,
2417+
Fortran::parser::OmpClause::HasDeviceAddr,
2418+
Fortran::parser::OmpClause::Reduction,
2419+
Fortran::parser::OmpClause::InReduction,
2420+
Fortran::parser::OmpClause::Allocate,
2421+
Fortran::parser::OmpClause::UsesAllocators,
2422+
Fortran::parser::OmpClause::Defaultmap>(
2423+
currentLocation, llvm::omp::Directive::OMPD_target);
23732424

23742425
llvm::SmallVector<mlir::Attribute> mapTypesAttr(mapTypes.begin(),
23752426
mapTypes.end());
@@ -2402,8 +2453,8 @@ genTeamsOp(Fortran::lower::AbstractConverter &converter,
24022453
cp.processDefault();
24032454
cp.processNumTeams(stmtCtx, numTeamsClauseOperand);
24042455
cp.processThreadLimit(stmtCtx, threadLimitClauseOperand);
2405-
if (cp.processReduction(currentLocation, reductionVars, reductionDeclSymbols))
2406-
TODO(currentLocation, "Reduction of TEAMS directive");
2456+
cp.processTODO<Fortran::parser::OmpClause::Reduction>(
2457+
currentLocation, llvm::omp::Directive::OMPD_teams);
24072458

24082459
return genOpWithBody<mlir::omp::TeamsOp>(
24092460
converter, eval, currentLocation, outerCombined, &clauseList,
@@ -2420,10 +2471,11 @@ genTeamsOp(Fortran::lower::AbstractConverter &converter,
24202471
// genOMP() Code generation helper functions
24212472
//===----------------------------------------------------------------------===//
24222473

2423-
static void genOMP(Fortran::lower::AbstractConverter &converter,
2424-
Fortran::lower::pft::Evaluation &eval,
2425-
const Fortran::parser::OpenMPSimpleStandaloneConstruct
2426-
&simpleStandaloneConstruct) {
2474+
static void
2475+
genOmpSimpleStandalone(Fortran::lower::AbstractConverter &converter,
2476+
Fortran::lower::pft::Evaluation &eval,
2477+
const Fortran::parser::OpenMPSimpleStandaloneConstruct
2478+
&simpleStandaloneConstruct) {
24272479
const auto &directive =
24282480
std::get<Fortran::parser::OmpSimpleStandaloneDirective>(
24292481
simpleStandaloneConstruct.t);
@@ -2439,6 +2491,10 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
24392491
firOpBuilder.create<mlir::omp::BarrierOp>(currentLocation);
24402492
break;
24412493
case llvm::omp::Directive::OMPD_taskwait:
2494+
ClauseProcessor(converter, opClauseList)
2495+
.processTODO<Fortran::parser::OmpClause::Depend,
2496+
Fortran::parser::OmpClause::Nowait>(
2497+
currentLocation, llvm::omp::Directive::OMPD_taskwait);
24422498
firOpBuilder.create<mlir::omp::TaskwaitOp>(currentLocation);
24432499
break;
24442500
case llvm::omp::Directive::OMPD_taskyield:
@@ -2462,6 +2518,24 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
24622518
}
24632519
}
24642520

2521+
static void
2522+
genOmpFlush(Fortran::lower::AbstractConverter &converter,
2523+
Fortran::lower::pft::Evaluation &eval,
2524+
const Fortran::parser::OpenMPFlushConstruct &flushConstruct) {
2525+
llvm::SmallVector<mlir::Value, 4> operandRange;
2526+
if (const auto &ompObjectList =
2527+
std::get<std::optional<Fortran::parser::OmpObjectList>>(
2528+
flushConstruct.t))
2529+
genObjectList(*ompObjectList, converter, operandRange);
2530+
const auto &memOrderClause =
2531+
std::get<std::optional<std::list<Fortran::parser::OmpMemoryOrderClause>>>(
2532+
flushConstruct.t);
2533+
if (memOrderClause && memOrderClause->size() > 0)
2534+
TODO(converter.getCurrentLocation(), "Handle OmpMemoryOrderClause");
2535+
converter.getFirOpBuilder().create<mlir::omp::FlushOp>(
2536+
converter.getCurrentLocation(), operandRange);
2537+
}
2538+
24652539
static void
24662540
genOMP(Fortran::lower::AbstractConverter &converter,
24672541
Fortran::lower::pft::Evaluation &eval,
@@ -2470,22 +2544,10 @@ genOMP(Fortran::lower::AbstractConverter &converter,
24702544
Fortran::common::visitors{
24712545
[&](const Fortran::parser::OpenMPSimpleStandaloneConstruct
24722546
&simpleStandaloneConstruct) {
2473-
genOMP(converter, eval, simpleStandaloneConstruct);
2547+
genOmpSimpleStandalone(converter, eval, simpleStandaloneConstruct);
24742548
},
24752549
[&](const Fortran::parser::OpenMPFlushConstruct &flushConstruct) {
2476-
llvm::SmallVector<mlir::Value, 4> operandRange;
2477-
if (const auto &ompObjectList =
2478-
std::get<std::optional<Fortran::parser::OmpObjectList>>(
2479-
flushConstruct.t))
2480-
genObjectList(*ompObjectList, converter, operandRange);
2481-
const auto &memOrderClause = std::get<std::optional<
2482-
std::list<Fortran::parser::OmpMemoryOrderClause>>>(
2483-
flushConstruct.t);
2484-
if (memOrderClause && memOrderClause->size() > 0)
2485-
TODO(converter.getCurrentLocation(),
2486-
"Handle OmpMemoryOrderClause");
2487-
converter.getFirOpBuilder().create<mlir::omp::FlushOp>(
2488-
converter.getCurrentLocation(), operandRange);
2550+
genOmpFlush(converter, eval, flushConstruct);
24892551
},
24902552
[&](const Fortran::parser::OpenMPCancelConstruct &cancelConstruct) {
24912553
TODO(converter.getCurrentLocation(), "OpenMPCancelConstruct");
@@ -2503,14 +2565,13 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
25032565
const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
25042566
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
25052567
llvm::SmallVector<mlir::Value> lowerBound, upperBound, step, linearVars,
2506-
linearStepVars, reductionVars, alignedVars, nontemporalVars;
2507-
mlir::Value scheduleChunkClauseOperand, ifClauseOperand;
2568+
linearStepVars, reductionVars;
2569+
mlir::Value scheduleChunkClauseOperand;
25082570
mlir::IntegerAttr orderedClauseOperand;
25092571
mlir::omp::ClauseOrderKindAttr orderClauseOperand;
25102572
mlir::omp::ClauseScheduleKindAttr scheduleValClauseOperand;
25112573
mlir::omp::ScheduleModifierAttr scheduleModClauseOperand;
25122574
mlir::UnitAttr nowaitClauseOperand, scheduleSimdClauseOperand;
2513-
mlir::IntegerAttr simdlenClauseOperand, safelenClauseOperand;
25142575
llvm::SmallVector<mlir::Attribute> reductionDeclSymbols;
25152576
Fortran::lower::StatementContext stmtCtx;
25162577
std::size_t loopVarTypeSize;
@@ -2570,12 +2631,10 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
25702631
cp.processCollapse(currentLocation, eval, lowerBound, upperBound, step, iv,
25712632
loopVarTypeSize);
25722633
cp.processScheduleChunk(stmtCtx, scheduleChunkClauseOperand);
2573-
cp.processIf(stmtCtx,
2574-
Fortran::parser::OmpIfClause::DirectiveNameModifier::Simd,
2575-
ifClauseOperand);
25762634
cp.processReduction(currentLocation, reductionVars, reductionDeclSymbols);
2577-
cp.processSimdlen(simdlenClauseOperand);
2578-
cp.processSafelen(safelenClauseOperand);
2635+
cp.processTODO<Fortran::parser::OmpClause::Linear,
2636+
Fortran::parser::OmpClause::Order>(currentLocation,
2637+
ompDirective);
25792638

25802639
// The types of lower bound, upper bound, and step are converted into the
25812640
// type of the loop variable if necessary.
@@ -2590,8 +2649,20 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
25902649
}
25912650

25922651
// 2.9.3.1 SIMD construct
2593-
// TODO: Support all the clauses
25942652
if (llvm::omp::allSimdSet.test(ompDirective)) {
2653+
llvm::SmallVector<mlir::Value> alignedVars, nontemporalVars;
2654+
mlir::Value ifClauseOperand;
2655+
mlir::IntegerAttr simdlenClauseOperand, safelenClauseOperand;
2656+
cp.processIf(stmtCtx,
2657+
Fortran::parser::OmpIfClause::DirectiveNameModifier::Simd,
2658+
ifClauseOperand);
2659+
cp.processSimdlen(simdlenClauseOperand);
2660+
cp.processSafelen(safelenClauseOperand);
2661+
cp.processTODO<Fortran::parser::OmpClause::Aligned,
2662+
Fortran::parser::OmpClause::Allocate,
2663+
Fortran::parser::OmpClause::Nontemporal>(currentLocation,
2664+
ompDirective);
2665+
25952666
mlir::TypeRange resultType;
25962667
auto simdLoopOp = firOpBuilder.create<mlir::omp::SimdLoopOp>(
25972668
currentLocation, resultType, lowerBound, upperBound, step, alignedVars,
@@ -2604,9 +2675,6 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
26042675
return;
26052676
}
26062677

2607-
// FIXME: Add support for following clauses:
2608-
// 1. linear
2609-
// 2. order
26102678
auto wsLoopOp = firOpBuilder.create<mlir::omp::WsLoopOp>(
26112679
currentLocation, lowerBound, upperBound, step, linearVars, linearStepVars,
26122680
reductionVars,
@@ -3340,6 +3408,9 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
33403408
cp.processTo(symbolAndClause);
33413409
cp.processLink(symbolAndClause);
33423410
cp.processDeviceType(deviceType);
3411+
cp.processTODO<Fortran::parser::OmpClause::Indirect>(
3412+
converter.getCurrentLocation(),
3413+
llvm::omp::Directive::OMPD_declare_target);
33433414
}
33443415

33453416
for (const DeclareTargetCapturePair &symClause : symbolAndClause) {

‎flang/test/Lower/OpenMP/Todo/firstprivate-target.f90

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
2+
! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
3+
4+
integer :: i
5+
! CHECK: not yet implemented: Unhandled clause FIRSTPRIVATE in TARGET construct
6+
!$omp target firstprivate(i)
7+
!$omp end target
8+
9+
end program

‎flang/test/Lower/OpenMP/Todo/reduction-teams.f90

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
22
! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
33

4-
! CHECK: not yet implemented: Reduction of TEAMS directive
4+
! CHECK: not yet implemented: Unhandled clause REDUCTION in TEAMS construct
55
subroutine reduction_teams()
66
integer :: i
77
i = 0

0 commit comments

Comments
 (0)