Skip to content

Commit 8650c80

Browse files
authored
[flang][OpenMP] Do not skip privatization of linear variable if it is OmpPreDetermined (#144315)
Current implementation of linear clause skips privatisation of all linear variables during the FIR generation phase, since linear variables are handled in their entirety by the OpenMP IRBuilder. However, "implicit" linear variables (like OmpPreDetermined) cannot be skipped, since FIR generation requires privatized symbols. This patch adds checks to skip the same. Fixes #142935
1 parent e8be733 commit 8650c80

File tree

1 file changed

+38
-2
lines changed

1 file changed

+38
-2
lines changed

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,42 @@ void DataSharingProcessor::collectOmpObjectListSymbol(
204204
}
205205

206206
void DataSharingProcessor::collectSymbolsForPrivatization() {
207+
// Add checks here for exceptional cases where privatization is not
208+
// needed and be deferred to a later phase (like OpenMP IRBuilder).
209+
// Such cases are suggested to be clearly documented and explained
210+
// instead of being silently skipped
211+
auto isException = [&](const Fortran::semantics::Symbol *sym) -> bool {
212+
// `OmpPreDetermined` symbols cannot be exceptions since
213+
// their privatized symbols are heavily used in FIR.
214+
if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined))
215+
return false;
216+
217+
// The handling of linear clause is deferred to the OpenMP
218+
// IRBuilder which is responsible for all its aspects,
219+
// including privatization. Privatizing linear variables at this point would
220+
// cause the following structure:
221+
//
222+
// omp.op linear(%linear = %step : !fir.ref<type>) {
223+
// Use %linear in this BB
224+
// }
225+
//
226+
// to be changed to the following:
227+
//
228+
// omp. op linear(%linear = %step : !fir.ref<type>)
229+
// private(%linear -> %arg0 : !fir.ref<i32>) {
230+
// Declare and use %arg0 in this BB
231+
// }
232+
//
233+
// The OpenMP IRBuilder needs to map the linear MLIR value
234+
// (i.e. %linear) to its `uses` in the BB to correctly
235+
// implement the functionalities of linear clause. However,
236+
// privatizing here disallows the IRBuilder to
237+
// draw a relation between %linear and %arg0. Hence skip.
238+
if (sym->test(Fortran::semantics::Symbol::Flag::OmpLinear))
239+
return true;
240+
return false;
241+
};
242+
207243
for (const omp::Clause &clause : clauses) {
208244
if (const auto &privateClause =
209245
std::get_if<omp::clause::Private>(&clause.u)) {
@@ -222,10 +258,10 @@ void DataSharingProcessor::collectSymbolsForPrivatization() {
222258
}
223259

224260
// TODO For common blocks, add the underlying objects within the block. Doing
225-
// so, we won't need to explicitely handle block objects (or forget to do
261+
// so, we won't need to explicitly handle block objects (or forget to do
226262
// so).
227263
for (auto *sym : explicitlyPrivatizedSymbols)
228-
if (!sym->test(Fortran::semantics::Symbol::Flag::OmpLinear))
264+
if (!isException(sym))
229265
allPrivatizedSymbols.insert(sym);
230266
}
231267

0 commit comments

Comments
 (0)