Skip to content

Commit ab19c32

Browse files
skatrakskatrak
committed
[Flang][OpenMP] Prevent re-composition of composite constructs
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.
1 parent 2b077ed commit ab19c32

File tree

7 files changed

+103
-488
lines changed

7 files changed

+103
-488
lines changed

‎flang/lib/Lower/OpenMP/Decomposer.cpp

Lines changed: 22 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,33 @@ 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;
12296
}
12397

124-
return constructs;
98+
return decompose.output;
99+
}
100+
101+
bool matchLeafSequence(ConstructQueue::const_iterator item,
102+
const ConstructQueue &queue,
103+
llvm::ArrayRef<llvm::omp::Directive> directives) {
104+
for (auto [dir, leaf] :
105+
llvm::zip_longest(directives, llvm::make_range(item, queue.end()))) {
106+
if (!dir || !leaf)
107+
return false;
108+
109+
if (dir.value() != leaf.value().id)
110+
return false;
111+
}
112+
return true;
125113
}
126114

127115
bool isLastItemInQueue(ConstructQueue::const_iterator item,

‎flang/lib/Lower/OpenMP/Decomposer.h

Lines changed: 6 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,12 @@ ConstructQueue buildConstructQueue(mlir::ModuleOp modOp,
4948

5049
bool isLastItemInQueue(ConstructQueue::const_iterator item,
5150
const ConstructQueue &queue);
51+
52+
/// Try to match a sequence of \c directives to the range of leaf constructs
53+
/// starting from \c item to the end of the \c queue.
54+
bool matchLeafSequence(ConstructQueue::const_iterator item,
55+
const ConstructQueue &queue,
56+
llvm::ArrayRef<llvm::omp::Directive> directives);
5257
} // namespace Fortran::lower::omp
5358

5459
#endif // FORTRAN_LOWER_OPENMP_DECOMPOSER_H

‎flang/lib/Lower/OpenMP/OpenMP.cpp

Lines changed: 70 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,6 +2042,7 @@ static void genCompositeDistributeParallelDoSimd(
20422042
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
20432043
mlir::Location loc, const ConstructQueue &queue,
20442044
ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
2045+
assert(std::distance(item, queue.end()) == 4 && "Invalid leaf constructs");
20452046
TODO(loc, "Composite DISTRIBUTE PARALLEL DO SIMD");
20462047
}
20472048

@@ -2052,17 +2053,23 @@ static void genCompositeDistributeSimd(
20522053
ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
20532054
lower::StatementContext stmtCtx;
20542055

2056+
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
2057+
ConstructQueue::const_iterator distributeItem = item;
2058+
ConstructQueue::const_iterator simdItem = std::next(distributeItem);
2059+
20552060
// Clause processing.
20562061
mlir::omp::DistributeOperands distributeClauseOps;
2057-
genDistributeClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
2058-
distributeClauseOps);
2062+
genDistributeClauses(converter, semaCtx, stmtCtx, distributeItem->clauses,
2063+
loc, distributeClauseOps);
20592064

20602065
mlir::omp::SimdOperands simdClauseOps;
2061-
genSimdClauses(converter, semaCtx, item->clauses, loc, simdClauseOps);
2066+
genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps);
20622067

2068+
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
2069+
// is placed by construct decomposition.
20632070
mlir::omp::LoopNestOperands loopNestClauseOps;
20642071
llvm::SmallVector<const semantics::Symbol *> iv;
2065-
genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
2072+
genLoopNestClauses(converter, semaCtx, eval, simdItem->clauses, loc,
20662073
loopNestClauseOps, iv);
20672074

20682075
// Operation creation.
@@ -2086,7 +2093,7 @@ static void genCompositeDistributeSimd(
20862093

20872094
assert(wrapperArgs.empty() &&
20882095
"Block args for omp.simd and omp.distribute currently not expected");
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.
@@ -2135,7 +2148,7 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
21352148

21362149
assert(wsloopReductionSyms.size() == wrapperArgs.size() &&
21372150
"Number of symbols and wrapper block arguments must match");
2138-
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
2151+
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, simdItem,
21392152
loopNestClauseOps, iv, wsloopReductionSyms, wrapperArgs,
21402153
llvm::omp::Directive::OMPD_do_simd, dsp);
21412154
}
@@ -2145,13 +2158,50 @@ static void genCompositeTaskloopSimd(
21452158
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
21462159
mlir::Location loc, const ConstructQueue &queue,
21472160
ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
2161+
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
21482162
TODO(loc, "Composite TASKLOOP SIMD");
21492163
}
21502164

21512165
//===----------------------------------------------------------------------===//
21522166
// Dispatch
21532167
//===----------------------------------------------------------------------===//
21542168

2169+
static bool genOMPCompositeDispatch(
2170+
lower::AbstractConverter &converter, lower::SymMap &symTable,
2171+
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
2172+
mlir::Location loc, const ConstructQueue &queue,
2173+
ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
2174+
using llvm::omp::Directive;
2175+
using llvm::omp::getLeafConstructs, lower::omp::matchLeafSequence;
2176+
2177+
if (matchLeafSequence(
2178+
item, queue,
2179+
getLeafConstructs(Directive::OMPD_distribute_parallel_do)))
2180+
genCompositeDistributeParallelDo(converter, symTable, semaCtx, eval, loc,
2181+
queue, item, dsp);
2182+
else if (matchLeafSequence(
2183+
item, queue,
2184+
getLeafConstructs(Directive::OMPD_distribute_parallel_do_simd)))
2185+
genCompositeDistributeParallelDoSimd(converter, symTable, semaCtx, eval,
2186+
loc, queue, item, dsp);
2187+
else if (matchLeafSequence(
2188+
item, queue, getLeafConstructs(Directive::OMPD_distribute_simd)))
2189+
genCompositeDistributeSimd(converter, symTable, semaCtx, eval, loc, queue,
2190+
item, dsp);
2191+
else if (matchLeafSequence(item, queue,
2192+
getLeafConstructs(Directive::OMPD_do_simd)))
2193+
genCompositeDoSimd(converter, symTable, semaCtx, eval, loc, queue, item,
2194+
dsp);
2195+
else if (matchLeafSequence(item, queue,
2196+
getLeafConstructs(Directive::OMPD_taskloop_simd)))
2197+
genCompositeTaskloopSimd(converter, symTable, semaCtx, eval, loc, queue,
2198+
item, dsp);
2199+
else
2200+
return false;
2201+
2202+
return true;
2203+
}
2204+
21552205
static void genOMPDispatch(lower::AbstractConverter &converter,
21562206
lower::SymMap &symTable,
21572207
semantics::SemanticsContext &semaCtx,
@@ -2165,10 +2215,18 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
21652215
llvm::omp::Association::Loop;
21662216
if (loopLeaf) {
21672217
symTable.pushScope();
2218+
// TODO: Use one DataSharingProcessor for each leaf of a composite
2219+
// construct.
21682220
loopDsp.emplace(converter, semaCtx, item->clauses, eval,
21692221
/*shouldCollectPreDeterminedSymbols=*/true,
21702222
/*useDelayedPrivatization=*/false, &symTable);
21712223
loopDsp->processStep1();
2224+
2225+
if (genOMPCompositeDispatch(converter, symTable, semaCtx, eval, loc, queue,
2226+
item, *loopDsp)) {
2227+
symTable.popScope();
2228+
return;
2229+
}
21722230
}
21732231

21742232
switch (llvm::omp::Directive dir = item->id) {
@@ -2267,24 +2325,13 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
22672325

22682326
// Composite constructs
22692327
case llvm::omp::Directive::OMPD_distribute_parallel_do:
2270-
genCompositeDistributeParallelDo(converter, symTable, semaCtx, eval, loc,
2271-
queue, item, *loopDsp);
2272-
break;
22732328
case llvm::omp::Directive::OMPD_distribute_parallel_do_simd:
2274-
genCompositeDistributeParallelDoSimd(converter, symTable, semaCtx, eval,
2275-
loc, queue, item, *loopDsp);
2276-
break;
22772329
case llvm::omp::Directive::OMPD_distribute_simd:
2278-
genCompositeDistributeSimd(converter, symTable, semaCtx, eval, loc, queue,
2279-
item, *loopDsp);
2280-
break;
22812330
case llvm::omp::Directive::OMPD_do_simd:
2282-
genCompositeDoSimd(converter, symTable, semaCtx, eval, loc, queue, item,
2283-
*loopDsp);
2284-
break;
22852331
case llvm::omp::Directive::OMPD_taskloop_simd:
2286-
genCompositeTaskloopSimd(converter, symTable, semaCtx, eval, loc, queue,
2287-
item, *loopDsp);
2332+
// Composite constructs should have been split into a sequence of leaf
2333+
// constructs and lowered by genOMPCompositeDispatch().
2334+
llvm_unreachable("Unexpected composite construct.");
22882335
break;
22892336
default:
22902337
break;

‎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)