Skip to content

Commit

Permalink
[Flang][OpenMP][Lower] Fail compilation with TODO errors for unsuppor…
Browse files Browse the repository at this point in the history 
…ted 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
  • Loading branch information
@skatrak
skatrak committed Aug 22, 2023
1 parent 048b506 commit 841c4dc
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 39 deletions.
147 changes: 109 additions & 38 deletions flang/lib/Lower/OpenMP.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Builder/Todo.h"
#include "flang/Optimizer/HLFIR/HLFIROps.h"
#include "flang/Parser/dump-parse-tree.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/openmp-directive-sets.h"
#include "flang/Semantics/tools.h"
Expand Down Expand Up @@ -549,15 +550,23 @@ class ClauseProcessor {
llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
&useDeviceSymbols) const;

// Call this method for these clauses that should be supported but are not
// implemented yet. It triggers a compilation error if any of the given
// clauses is found.
template <typename... Ts>
void processTODO(mlir::Location currentLocation,
llvm::omp::Directive directive) const;

private:
using ClauseIterator = std::list<ClauseTy>::const_iterator;

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

return end;
}
Expand Down Expand Up @@ -1773,6 +1782,24 @@ bool ClauseProcessor::processUseDevicePtr(
});
}

template <typename... Ts>
void ClauseProcessor::processTODO(mlir::Location currentLocation,
llvm::omp::Directive directive) const {
auto checkUnhandledClause = [&](const auto *x) {
if (!x)
return;
TODO(currentLocation,
"Unhandled clause " +
llvm::StringRef(Fortran::parser::ParseTreeDumper::GetNodeName(*x))
.upper() +
" in " + llvm::omp::getOpenMPDirectiveName(directive).upper() +
" construct");
};

for (ClauseIterator it = clauses.v.begin(); it != clauses.v.end(); ++it)
(checkUnhandledClause(std::get_if<Ts>(&it->u)), ...);
}

//===----------------------------------------------------------------------===//
// Code generation helper functions
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -2213,8 +2240,11 @@ genSingleOp(Fortran::lower::AbstractConverter &converter,
llvm::SmallVector<mlir::Value> allocateOperands, allocatorOperands;
mlir::UnitAttr nowaitAttr;

ClauseProcessor(converter, beginClauseList)
.processAllocate(allocatorOperands, allocateOperands);
ClauseProcessor cp(converter, beginClauseList);
cp.processAllocate(allocatorOperands, allocateOperands);
cp.processTODO<Fortran::parser::OmpClause::Copyprivate>(
currentLocation, llvm::omp::Directive::OMPD_single);

ClauseProcessor(converter, endClauseList).processNowait(nowaitAttr);

return genOpWithBody<mlir::omp::SingleOp>(
Expand Down Expand Up @@ -2244,6 +2274,10 @@ genTaskOp(Fortran::lower::AbstractConverter &converter,
cp.processMergeable(mergeableAttr);
cp.processPriority(stmtCtx, priorityClauseOperand);
cp.processDepend(dependTypeOperands, dependOperands);
cp.processTODO<Fortran::parser::OmpClause::InReduction,
Fortran::parser::OmpClause::Detach,
Fortran::parser::OmpClause::Affinity>(
currentLocation, llvm::omp::Directive::OMPD_task);

return genOpWithBody<mlir::omp::TaskOp>(
converter, eval, currentLocation, /*outerCombined=*/false, &clauseList,
Expand All @@ -2263,9 +2297,10 @@ genTaskGroupOp(Fortran::lower::AbstractConverter &converter,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList) {
llvm::SmallVector<mlir::Value> allocateOperands, allocatorOperands;
// TODO: Add task_reduction support
ClauseProcessor(converter, clauseList)
.processAllocate(allocatorOperands, allocateOperands);
ClauseProcessor cp(converter, clauseList);
cp.processAllocate(allocatorOperands, allocateOperands);
cp.processTODO<Fortran::parser::OmpClause::TaskReduction>(
currentLocation, llvm::omp::Directive::OMPD_taskgroup);
return genOpWithBody<mlir::omp::TaskGroupOp>(
converter, eval, currentLocation, /*outerCombined=*/false, &clauseList,
/*task_reduction_vars=*/mlir::ValueRange(),
Expand Down Expand Up @@ -2323,12 +2358,15 @@ genEnterExitDataOp(Fortran::lower::AbstractConverter &converter,
llvm::SmallVector<mlir::IntegerAttr> mapTypes;

Fortran::parser::OmpIfClause::DirectiveNameModifier directiveName;
llvm::omp::Directive directive;
if constexpr (std::is_same_v<OpTy, mlir::omp::EnterDataOp>) {
directiveName =
Fortran::parser::OmpIfClause::DirectiveNameModifier::TargetEnterData;
directive = llvm::omp::Directive::OMPD_target_enter_data;
} else if constexpr (std::is_same_v<OpTy, mlir::omp::ExitDataOp>) {
directiveName =
Fortran::parser::OmpIfClause::DirectiveNameModifier::TargetExitData;
directive = llvm::omp::Directive::OMPD_target_exit_data;
} else {
return nullptr;
}
Expand All @@ -2338,6 +2376,8 @@ genEnterExitDataOp(Fortran::lower::AbstractConverter &converter,
cp.processDevice(stmtCtx, deviceOperand);
cp.processNowait(nowaitAttr);
cp.processMap(mapOperands, mapTypes);
cp.processTODO<Fortran::parser::OmpClause::Depend>(currentLocation,
directive);

llvm::SmallVector<mlir::Attribute> mapTypesAttr(mapTypes.begin(),
mapTypes.end());
Expand Down Expand Up @@ -2370,6 +2410,17 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
cp.processThreadLimit(stmtCtx, threadLimitOperand);
cp.processNowait(nowaitAttr);
cp.processMap(mapOperands, mapTypes);
cp.processTODO<Fortran::parser::OmpClause::Private,
Fortran::parser::OmpClause::Depend,
Fortran::parser::OmpClause::Firstprivate,
Fortran::parser::OmpClause::IsDevicePtr,
Fortran::parser::OmpClause::HasDeviceAddr,
Fortran::parser::OmpClause::Reduction,
Fortran::parser::OmpClause::InReduction,
Fortran::parser::OmpClause::Allocate,
Fortran::parser::OmpClause::UsesAllocators,
Fortran::parser::OmpClause::Defaultmap>(
currentLocation, llvm::omp::Directive::OMPD_target);

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

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

static void genOMP(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPSimpleStandaloneConstruct
&simpleStandaloneConstruct) {
static void
genOmpSimpleStandalone(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPSimpleStandaloneConstruct
&simpleStandaloneConstruct) {
const auto &directive =
std::get<Fortran::parser::OmpSimpleStandaloneDirective>(
simpleStandaloneConstruct.t);
Expand All @@ -2439,6 +2491,10 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
firOpBuilder.create<mlir::omp::BarrierOp>(currentLocation);
break;
case llvm::omp::Directive::OMPD_taskwait:
ClauseProcessor(converter, opClauseList)
.processTODO<Fortran::parser::OmpClause::Depend,
Fortran::parser::OmpClause::Nowait>(
currentLocation, llvm::omp::Directive::OMPD_taskwait);
firOpBuilder.create<mlir::omp::TaskwaitOp>(currentLocation);
break;
case llvm::omp::Directive::OMPD_taskyield:
Expand All @@ -2462,6 +2518,24 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
}
}

static void
genOmpFlush(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPFlushConstruct &flushConstruct) {
llvm::SmallVector<mlir::Value, 4> operandRange;
if (const auto &ompObjectList =
std::get<std::optional<Fortran::parser::OmpObjectList>>(
flushConstruct.t))
genObjectList(*ompObjectList, converter, operandRange);
const auto &memOrderClause =
std::get<std::optional<std::list<Fortran::parser::OmpMemoryOrderClause>>>(
flushConstruct.t);
if (memOrderClause && memOrderClause->size() > 0)
TODO(converter.getCurrentLocation(), "Handle OmpMemoryOrderClause");
converter.getFirOpBuilder().create<mlir::omp::FlushOp>(
converter.getCurrentLocation(), operandRange);
}

static void
genOMP(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
Expand All @@ -2470,22 +2544,10 @@ genOMP(Fortran::lower::AbstractConverter &converter,
Fortran::common::visitors{
[&](const Fortran::parser::OpenMPSimpleStandaloneConstruct
&simpleStandaloneConstruct) {
genOMP(converter, eval, simpleStandaloneConstruct);
genOmpSimpleStandalone(converter, eval, simpleStandaloneConstruct);
},
[&](const Fortran::parser::OpenMPFlushConstruct &flushConstruct) {
llvm::SmallVector<mlir::Value, 4> operandRange;
if (const auto &ompObjectList =
std::get<std::optional<Fortran::parser::OmpObjectList>>(
flushConstruct.t))
genObjectList(*ompObjectList, converter, operandRange);
const auto &memOrderClause = std::get<std::optional<
std::list<Fortran::parser::OmpMemoryOrderClause>>>(
flushConstruct.t);
if (memOrderClause && memOrderClause->size() > 0)
TODO(converter.getCurrentLocation(),
"Handle OmpMemoryOrderClause");
converter.getFirOpBuilder().create<mlir::omp::FlushOp>(
converter.getCurrentLocation(), operandRange);
genOmpFlush(converter, eval, flushConstruct);
},
[&](const Fortran::parser::OpenMPCancelConstruct &cancelConstruct) {
TODO(converter.getCurrentLocation(), "OpenMPCancelConstruct");
Expand All @@ -2503,14 +2565,13 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
llvm::SmallVector<mlir::Value> lowerBound, upperBound, step, linearVars,
linearStepVars, reductionVars, alignedVars, nontemporalVars;
mlir::Value scheduleChunkClauseOperand, ifClauseOperand;
linearStepVars, reductionVars;
mlir::Value scheduleChunkClauseOperand;
mlir::IntegerAttr orderedClauseOperand;
mlir::omp::ClauseOrderKindAttr orderClauseOperand;
mlir::omp::ClauseScheduleKindAttr scheduleValClauseOperand;
mlir::omp::ScheduleModifierAttr scheduleModClauseOperand;
mlir::UnitAttr nowaitClauseOperand, scheduleSimdClauseOperand;
mlir::IntegerAttr simdlenClauseOperand, safelenClauseOperand;
llvm::SmallVector<mlir::Attribute> reductionDeclSymbols;
Fortran::lower::StatementContext stmtCtx;
std::size_t loopVarTypeSize;
Expand Down Expand Up @@ -2570,12 +2631,10 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
cp.processCollapse(currentLocation, eval, lowerBound, upperBound, step, iv,
loopVarTypeSize);
cp.processScheduleChunk(stmtCtx, scheduleChunkClauseOperand);
cp.processIf(stmtCtx,
Fortran::parser::OmpIfClause::DirectiveNameModifier::Simd,
ifClauseOperand);
cp.processReduction(currentLocation, reductionVars, reductionDeclSymbols);
cp.processSimdlen(simdlenClauseOperand);
cp.processSafelen(safelenClauseOperand);
cp.processTODO<Fortran::parser::OmpClause::Linear,
Fortran::parser::OmpClause::Order>(currentLocation,
ompDirective);

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

// 2.9.3.1 SIMD construct
// TODO: Support all the clauses
if (llvm::omp::allSimdSet.test(ompDirective)) {
llvm::SmallVector<mlir::Value> alignedVars, nontemporalVars;
mlir::Value ifClauseOperand;
mlir::IntegerAttr simdlenClauseOperand, safelenClauseOperand;
cp.processIf(stmtCtx,
Fortran::parser::OmpIfClause::DirectiveNameModifier::Simd,
ifClauseOperand);
cp.processSimdlen(simdlenClauseOperand);
cp.processSafelen(safelenClauseOperand);
cp.processTODO<Fortran::parser::OmpClause::Aligned,
Fortran::parser::OmpClause::Allocate,
Fortran::parser::OmpClause::Nontemporal>(currentLocation,
ompDirective);

mlir::TypeRange resultType;
auto simdLoopOp = firOpBuilder.create<mlir::omp::SimdLoopOp>(
currentLocation, resultType, lowerBound, upperBound, step, alignedVars,
Expand All @@ -2604,9 +2675,6 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
return;
}

// FIXME: Add support for following clauses:
// 1. linear
// 2. order
auto wsLoopOp = firOpBuilder.create<mlir::omp::WsLoopOp>(
currentLocation, lowerBound, upperBound, step, linearVars, linearStepVars,
reductionVars,
Expand Down Expand Up @@ -3340,6 +3408,9 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
cp.processTo(symbolAndClause);
cp.processLink(symbolAndClause);
cp.processDeviceType(deviceType);
cp.processTODO<Fortran::parser::OmpClause::Indirect>(
converter.getCurrentLocation(),
llvm::omp::Directive::OMPD_declare_target);
}

for (const DeclareTargetCapturePair &symClause : symbolAndClause) {
Expand Down
9 changes: 9 additions & 0 deletions flang/test/Lower/OpenMP/Todo/firstprivate-target.f90
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s

integer :: i
! CHECK: not yet implemented: Unhandled clause FIRSTPRIVATE in TARGET construct
!$omp target firstprivate(i)
!$omp end target

end program
2 changes: 1 addition & 1 deletion flang/test/Lower/OpenMP/Todo/reduction-teams.f90
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s

! CHECK: not yet implemented: Reduction of TEAMS directive
! CHECK: not yet implemented: Unhandled clause REDUCTION in TEAMS construct
subroutine reduction_teams()
integer :: i
i = 0
Expand Down

0 comments on commit 841c4dc

Please sign in to comment.