Skip to content

Commit aa875cf

Browse files
authored
[Flang][OpenMP] Prevent re-composition of composite constructs (#102613)
After decomposition of OpenMP compound constructs and assignment of applicable clauses to each leaf construct, composite constructs are then combined again into a single element in the construct queue. This helped later lowering stages easily identify composite constructs. However, as a result of the re-composition stage, the same list of clauses is used to produce all MLIR operations corresponding to each leaf of the original composite construct. This undoes existing logic introducing implicit clauses and deciding to which leaf construct(s) each clause applies. This patch removes construct re-composition logic and updates Flang lowering to be able to identify composite constructs from a list of leaf constructs. As a result, the right set of clauses is produced for each operation representing a leaf of a composite construct. PR stack: - #102612 - #102613
1 parent ba84cfb commit aa875cf

File tree

7 files changed

+105
-496
lines changed

7 files changed

+105
-496
lines changed

flang/lib/Lower/OpenMP/Decomposer.cpp

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include "llvm/ADT/STLExtras.h"
2323
#include "llvm/ADT/SmallVector.h"
2424
#include "llvm/Frontend/OpenMP/ClauseT.h"
25-
#include "llvm/Frontend/OpenMP/ConstructCompositionT.h"
2625
#include "llvm/Frontend/OpenMP/ConstructDecompositionT.h"
2726
#include "llvm/Frontend/OpenMP/OMP.h"
2827
#include "llvm/Support/raw_ostream.h"
@@ -68,12 +67,6 @@ struct ConstructDecomposition {
6867
};
6968
} // namespace
7069

71-
static UnitConstruct mergeConstructs(uint32_t version,
72-
llvm::ArrayRef<UnitConstruct> units) {
73-
tomp::ConstructCompositionT compose(version, units);
74-
return compose.merged;
75-
}
76-
7770
namespace Fortran::lower::omp {
7871
LLVM_DUMP_METHOD llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
7972
const UnitConstruct &uc) {
@@ -90,38 +83,37 @@ ConstructQueue buildConstructQueue(
9083
Fortran::lower::pft::Evaluation &eval, const parser::CharBlock &source,
9184
llvm::omp::Directive compound, const List<Clause> &clauses) {
9285

93-
List<UnitConstruct> constructs;
94-
9586
ConstructDecomposition decompose(modOp, semaCtx, eval, compound, clauses);
9687
assert(!decompose.output.empty() && "Construct decomposition failed");
9788

98-
llvm::SmallVector<llvm::omp::Directive> loweringUnits;
99-
std::ignore =
100-
llvm::omp::getLeafOrCompositeConstructs(compound, loweringUnits);
101-
uint32_t version = getOpenMPVersionAttribute(modOp);
102-
103-
int leafIndex = 0;
104-
for (llvm::omp::Directive dir_id : loweringUnits) {
105-
llvm::ArrayRef<llvm::omp::Directive> leafsOrSelf =
106-
llvm::omp::getLeafConstructsOrSelf(dir_id);
107-
size_t numLeafs = leafsOrSelf.size();
108-
109-
llvm::ArrayRef<UnitConstruct> toMerge{&decompose.output[leafIndex],
110-
numLeafs};
111-
auto &uc = constructs.emplace_back(mergeConstructs(version, toMerge));
112-
113-
if (!transferLocations(clauses, uc.clauses)) {
114-
// If some clauses are left without source information, use the
115-
// directive's source.
116-
for (auto &clause : uc.clauses) {
117-
if (clause.source.empty())
118-
clause.source = source;
119-
}
120-
}
121-
leafIndex += numLeafs;
89+
for (UnitConstruct &uc : decompose.output) {
90+
assert(getLeafConstructs(uc.id).empty() && "unexpected compound directive");
91+
// If some clauses are left without source information, use the directive's
92+
// source.
93+
for (auto &clause : uc.clauses)
94+
if (clause.source.empty())
95+
clause.source = source;
96+
}
97+
98+
return decompose.output;
99+
}
100+
101+
bool matchLeafSequence(ConstructQueue::const_iterator item,
102+
const ConstructQueue &queue,
103+
llvm::omp::Directive directive) {
104+
llvm::ArrayRef<llvm::omp::Directive> leafDirs =
105+
llvm::omp::getLeafConstructsOrSelf(directive);
106+
107+
for (auto [dir, leaf] :
108+
llvm::zip_longest(leafDirs, llvm::make_range(item, queue.end()))) {
109+
if (!dir.has_value() || !leaf.has_value())
110+
return false;
111+
112+
if (*dir != leaf->id)
113+
return false;
122114
}
123115

124-
return constructs;
116+
return true;
125117
}
126118

127119
bool isLastItemInQueue(ConstructQueue::const_iterator item,

flang/lib/Lower/OpenMP/Decomposer.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
#include "Clauses.h"
1212
#include "mlir/IR/BuiltinOps.h"
13-
#include "llvm/Frontend/OpenMP/ConstructCompositionT.h"
1413
#include "llvm/Frontend/OpenMP/ConstructDecompositionT.h"
1514
#include "llvm/Frontend/OpenMP/OMP.h"
1615
#include "llvm/Support/Compiler.h"
@@ -49,6 +48,15 @@ ConstructQueue buildConstructQueue(mlir::ModuleOp modOp,
4948

5049
bool isLastItemInQueue(ConstructQueue::const_iterator item,
5150
const ConstructQueue &queue);
51+
52+
/// Try to match the leaf constructs conforming the given \c directive to the
53+
/// range of leaf constructs starting from \c item to the end of the \c queue.
54+
/// If \c directive doesn't represent a compound directive, check that \c item
55+
/// matches that directive and is the only element before the end of the
56+
/// \c queue.
57+
bool matchLeafSequence(ConstructQueue::const_iterator item,
58+
const ConstructQueue &queue,
59+
llvm::omp::Directive directive);
5260
} // namespace Fortran::lower::omp
5361

5462
#endif // FORTRAN_LOWER_OPENMP_DECOMPOSER_H

flang/lib/Lower/OpenMP/OpenMP.cpp

Lines changed: 65 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2044,6 +2044,7 @@ static void genCompositeDistributeParallelDoSimd(
20442044
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
20452045
mlir::Location loc, const ConstructQueue &queue,
20462046
ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
2047+
assert(std::distance(item, queue.end()) == 4 && "Invalid leaf constructs");
20472048
TODO(loc, "Composite DISTRIBUTE PARALLEL DO SIMD");
20482049
}
20492050

@@ -2054,17 +2055,23 @@ static void genCompositeDistributeSimd(
20542055
ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
20552056
lower::StatementContext stmtCtx;
20562057

2058+
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
2059+
ConstructQueue::const_iterator distributeItem = item;
2060+
ConstructQueue::const_iterator simdItem = std::next(distributeItem);
2061+
20572062
// Clause processing.
20582063
mlir::omp::DistributeOperands distributeClauseOps;
2059-
genDistributeClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
2060-
distributeClauseOps);
2064+
genDistributeClauses(converter, semaCtx, stmtCtx, distributeItem->clauses,
2065+
loc, distributeClauseOps);
20612066

20622067
mlir::omp::SimdOperands simdClauseOps;
2063-
genSimdClauses(converter, semaCtx, item->clauses, loc, simdClauseOps);
2068+
genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps);
20642069

2070+
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
2071+
// is placed by construct decomposition.
20652072
mlir::omp::LoopNestOperands loopNestClauseOps;
20662073
llvm::SmallVector<const semantics::Symbol *> iv;
2067-
genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
2074+
genLoopNestClauses(converter, semaCtx, eval, simdItem->clauses, loc,
20682075
loopNestClauseOps, iv);
20692076

20702077
// Operation creation.
@@ -2086,7 +2093,7 @@ static void genCompositeDistributeSimd(
20862093
llvm::concat<mlir::BlockArgument>(distributeOp.getRegion().getArguments(),
20872094
simdOp.getRegion().getArguments()));
20882095

2089-
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
2096+
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, simdItem,
20902097
loopNestClauseOps, iv, /*wrapperSyms=*/{}, wrapperArgs,
20912098
llvm::omp::Directive::OMPD_distribute_simd, dsp);
20922099
}
@@ -2100,19 +2107,25 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
21002107
DataSharingProcessor &dsp) {
21012108
lower::StatementContext stmtCtx;
21022109

2110+
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
2111+
ConstructQueue::const_iterator doItem = item;
2112+
ConstructQueue::const_iterator simdItem = std::next(doItem);
2113+
21032114
// Clause processing.
21042115
mlir::omp::WsloopOperands wsloopClauseOps;
21052116
llvm::SmallVector<const semantics::Symbol *> wsloopReductionSyms;
21062117
llvm::SmallVector<mlir::Type> wsloopReductionTypes;
2107-
genWsloopClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
2118+
genWsloopClauses(converter, semaCtx, stmtCtx, doItem->clauses, loc,
21082119
wsloopClauseOps, wsloopReductionTypes, wsloopReductionSyms);
21092120

21102121
mlir::omp::SimdOperands simdClauseOps;
2111-
genSimdClauses(converter, semaCtx, item->clauses, loc, simdClauseOps);
2122+
genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps);
21122123

2124+
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
2125+
// is placed by construct decomposition.
21132126
mlir::omp::LoopNestOperands loopNestClauseOps;
21142127
llvm::SmallVector<const semantics::Symbol *> iv;
2115-
genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
2128+
genLoopNestClauses(converter, semaCtx, eval, simdItem->clauses, loc,
21162129
loopNestClauseOps, iv);
21172130

21182131
// Operation creation.
@@ -2133,7 +2146,7 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
21332146
auto wrapperArgs = llvm::to_vector(llvm::concat<mlir::BlockArgument>(
21342147
wsloopOp.getRegion().getArguments(), simdOp.getRegion().getArguments()));
21352148

2136-
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
2149+
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, simdItem,
21372150
loopNestClauseOps, iv, wsloopReductionSyms, wrapperArgs,
21382151
llvm::omp::Directive::OMPD_do_simd, dsp);
21392152
}
@@ -2143,13 +2156,44 @@ static void genCompositeTaskloopSimd(
21432156
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
21442157
mlir::Location loc, const ConstructQueue &queue,
21452158
ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
2159+
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
21462160
TODO(loc, "Composite TASKLOOP SIMD");
21472161
}
21482162

21492163
//===----------------------------------------------------------------------===//
21502164
// Dispatch
21512165
//===----------------------------------------------------------------------===//
21522166

2167+
static bool genOMPCompositeDispatch(
2168+
lower::AbstractConverter &converter, lower::SymMap &symTable,
2169+
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
2170+
mlir::Location loc, const ConstructQueue &queue,
2171+
ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
2172+
using llvm::omp::Directive;
2173+
using lower::omp::matchLeafSequence;
2174+
2175+
if (matchLeafSequence(item, queue, Directive::OMPD_distribute_parallel_do))
2176+
genCompositeDistributeParallelDo(converter, symTable, semaCtx, eval, loc,
2177+
queue, item, dsp);
2178+
else if (matchLeafSequence(item, queue,
2179+
Directive::OMPD_distribute_parallel_do_simd))
2180+
genCompositeDistributeParallelDoSimd(converter, symTable, semaCtx, eval,
2181+
loc, queue, item, dsp);
2182+
else if (matchLeafSequence(item, queue, Directive::OMPD_distribute_simd))
2183+
genCompositeDistributeSimd(converter, symTable, semaCtx, eval, loc, queue,
2184+
item, dsp);
2185+
else if (matchLeafSequence(item, queue, Directive::OMPD_do_simd))
2186+
genCompositeDoSimd(converter, symTable, semaCtx, eval, loc, queue, item,
2187+
dsp);
2188+
else if (matchLeafSequence(item, queue, Directive::OMPD_taskloop_simd))
2189+
genCompositeTaskloopSimd(converter, symTable, semaCtx, eval, loc, queue,
2190+
item, dsp);
2191+
else
2192+
return false;
2193+
2194+
return true;
2195+
}
2196+
21532197
static void genOMPDispatch(lower::AbstractConverter &converter,
21542198
lower::SymMap &symTable,
21552199
semantics::SemanticsContext &semaCtx,
@@ -2163,10 +2207,18 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
21632207
llvm::omp::Association::Loop;
21642208
if (loopLeaf) {
21652209
symTable.pushScope();
2210+
// TODO: Use one DataSharingProcessor for each leaf of a composite
2211+
// construct.
21662212
loopDsp.emplace(converter, semaCtx, item->clauses, eval,
21672213
/*shouldCollectPreDeterminedSymbols=*/true,
21682214
/*useDelayedPrivatization=*/false, &symTable);
21692215
loopDsp->processStep1();
2216+
2217+
if (genOMPCompositeDispatch(converter, symTable, semaCtx, eval, loc, queue,
2218+
item, *loopDsp)) {
2219+
symTable.popScope();
2220+
return;
2221+
}
21702222
}
21712223

21722224
switch (llvm::omp::Directive dir = item->id) {
@@ -2262,29 +2314,11 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
22622314
// that use this construct, add a single construct for now.
22632315
genSingleOp(converter, symTable, semaCtx, eval, loc, queue, item);
22642316
break;
2265-
2266-
// Composite constructs
2267-
case llvm::omp::Directive::OMPD_distribute_parallel_do:
2268-
genCompositeDistributeParallelDo(converter, symTable, semaCtx, eval, loc,
2269-
queue, item, *loopDsp);
2270-
break;
2271-
case llvm::omp::Directive::OMPD_distribute_parallel_do_simd:
2272-
genCompositeDistributeParallelDoSimd(converter, symTable, semaCtx, eval,
2273-
loc, queue, item, *loopDsp);
2274-
break;
2275-
case llvm::omp::Directive::OMPD_distribute_simd:
2276-
genCompositeDistributeSimd(converter, symTable, semaCtx, eval, loc, queue,
2277-
item, *loopDsp);
2278-
break;
2279-
case llvm::omp::Directive::OMPD_do_simd:
2280-
genCompositeDoSimd(converter, symTable, semaCtx, eval, loc, queue, item,
2281-
*loopDsp);
2282-
break;
2283-
case llvm::omp::Directive::OMPD_taskloop_simd:
2284-
genCompositeTaskloopSimd(converter, symTable, semaCtx, eval, loc, queue,
2285-
item, *loopDsp);
2286-
break;
22872317
default:
2318+
// Combined and composite constructs should have been split into a sequence
2319+
// of leaf constructs when building the construct queue.
2320+
assert(!llvm::omp::isLeafConstruct(dir) &&
2321+
"Unexpected compound construct.");
22882322
break;
22892323
}
22902324

flang/test/Lower/OpenMP/Todo/omp-do-simd-linear.f90

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
55
subroutine testDoSimdLinear(int_array)
66
integer :: int_array(*)
7-
!CHECK: not yet implemented: Unhandled clause LINEAR in DO construct
7+
!CHECK: not yet implemented: Unhandled clause LINEAR in SIMD construct
88
!$omp do simd linear(int_array)
99
do index_ = 1, 10
1010
end do

flang/test/Lower/OpenMP/default-clause-byref.f90

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,9 @@ subroutine nested_default_clause_tests
197197
!CHECK: %[[Y_DECL:.*]]:2 = hlfir.declare %[[Y]] {uniq_name = "_QFnested_default_clause_testsEy"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
198198
!CHECK: %[[Z:.*]] = fir.alloca i32 {bindc_name = "z", uniq_name = "_QFnested_default_clause_testsEz"}
199199
!CHECK: %[[Z_DECL:.*]]:2 = hlfir.declare %[[Z]] {uniq_name = "_QFnested_default_clause_testsEz"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
200-
!CHECK: omp.parallel private({{.*}} {{.*}}#0 -> %[[PRIVATE_Y:.*]] : {{.*}}, {{.*firstprivate.*}} {{.*}}#0 -> %[[PRIVATE_X:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_Z:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_K:.*]] : {{.*}}) {
201-
!CHECK: %[[PRIVATE_Y_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Y]] {uniq_name = "_QFnested_default_clause_testsEy"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
200+
!CHECK: omp.parallel private({{.*firstprivate.*}} {{.*}}#0 -> %[[PRIVATE_X:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_Y:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_Z:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_K:.*]] : {{.*}}) {
202201
!CHECK: %[[PRIVATE_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
202+
!CHECK: %[[PRIVATE_Y_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Y]] {uniq_name = "_QFnested_default_clause_testsEy"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
203203
!CHECK: %[[PRIVATE_Z_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Z]] {uniq_name = "_QFnested_default_clause_testsEz"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
204204
!CHECK: %[[PRIVATE_K_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_K]] {uniq_name = "_QFnested_default_clause_testsEk"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
205205
!CHECK: omp.parallel private({{.*}} {{.*}}#0 -> %[[INNER_PRIVATE_Y:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[INNER_PRIVATE_X:.*]] : {{.*}}) {

flang/test/Lower/OpenMP/default-clause.f90

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,9 @@ end program default_clause_lowering
134134
!CHECK: %[[Y_DECL:.*]]:2 = hlfir.declare %[[Y]] {uniq_name = "_QFnested_default_clause_test1Ey"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
135135
!CHECK: %[[Z:.*]] = fir.alloca i32 {bindc_name = "z", uniq_name = "_QFnested_default_clause_test1Ez"}
136136
!CHECK: %[[Z_DECL:.*]]:2 = hlfir.declare %[[Z]] {uniq_name = "_QFnested_default_clause_test1Ez"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
137-
!CHECK: omp.parallel private({{.*}} {{.*}}#0 -> %[[PRIVATE_Y:.*]] : {{.*}}, {{.*firstprivate.*}} {{.*}}#0 -> %[[PRIVATE_X:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_Z:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_K:.*]] : {{.*}}) {
138-
!CHECK: %[[PRIVATE_Y_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Y]] {uniq_name = "_QFnested_default_clause_test1Ey"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
137+
!CHECK: omp.parallel private({{.*firstprivate.*}} {{.*}}#0 -> %[[PRIVATE_X:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_Y:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_Z:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[PRIVATE_K:.*]] : {{.*}}) {
139138
!CHECK: %[[PRIVATE_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_X]] {uniq_name = "_QFnested_default_clause_test1Ex"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
139+
!CHECK: %[[PRIVATE_Y_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Y]] {uniq_name = "_QFnested_default_clause_test1Ey"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
140140
!CHECK: %[[PRIVATE_Z_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Z]] {uniq_name = "_QFnested_default_clause_test1Ez"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
141141
!CHECK: %[[PRIVATE_K_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_K]] {uniq_name = "_QFnested_default_clause_test1Ek"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
142142
!CHECK: omp.parallel private({{.*}} {{.*}}#0 -> %[[INNER_PRIVATE_Y:.*]] : {{.*}}, {{.*}} {{.*}}#0 -> %[[INNER_PRIVATE_X:.*]] : {{.*}}) {

0 commit comments

Comments
 (0)