Skip to content

Commit a8a797f

Browse files
committed
[flang][OpenMP] Delayed privatization for variable with equivalence association
Handles variables that are storage associated via `equivalence`. The problem is that these variables are declared as `fir.ptr`s while their privatized storage is declared as `fir.ref` which was triggering a validation error in the OpenMP dialect. So far, I just disabled a small part of the validation logic until we find a way to properly check of compatible types without leaking the FIR dialect specifics in the OpenMP dialect validation logic.
1 parent 91450f1 commit a8a797f

File tree

5 files changed

+90
-11
lines changed

5 files changed

+90
-11
lines changed

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
523523
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
524524

525525
symTable->pushScope();
526+
mlir::Type yieldedType;
526527

527528
// Populate the `alloc` region.
528529
{
@@ -542,9 +543,10 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
542543
symTable->addSymbol(*sym, localExV);
543544
symTable->pushScope();
544545
cloneSymbol(sym);
545-
firOpBuilder.create<mlir::omp::YieldOp>(
546+
auto yieldOp = firOpBuilder.create<mlir::omp::YieldOp>(
546547
hsb.getAddr().getLoc(),
547548
symTable->shallowLookupSymbol(*sym).getAddr());
549+
yieldedType = yieldOp.getOperand(0).getType();
548550
symTable->popScope();
549551
}
550552

@@ -553,8 +555,9 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
553555
mlir::Region &copyRegion = result.getCopyRegion();
554556
// First block argument corresponding to the original/host value while
555557
// second block argument corresponding to the privatized value.
556-
mlir::Block *copyEntryBlock = firOpBuilder.createBlock(
557-
&copyRegion, /*insertPt=*/{}, {symType, symType}, {symLoc, symLoc});
558+
mlir::Block *copyEntryBlock =
559+
firOpBuilder.createBlock(&copyRegion, /*insertPt=*/{},
560+
{symType, yieldedType}, {symLoc, symLoc});
558561
firOpBuilder.setInsertionPointToEnd(copyEntryBlock);
559562

560563
auto addSymbol = [&](unsigned argIdx, bool force = false) {
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
! Test delayed privatization for variables that are storage associated via `EQUIVALENCE`.
2+
3+
! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
4+
! RUN: -o - %s 2>&1 | FileCheck %s
5+
! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 \
6+
! RUN: | FileCheck %s
7+
8+
subroutine private_common
9+
real x, y
10+
equivalence (x,y)
11+
!$omp parallel firstprivate(x)
12+
x = 3.14
13+
!$omp end parallel
14+
end subroutine
15+
16+
! CHECK: omp.private {type = firstprivate} @[[X_PRIVATIZER:.*]] : ![[X_TYPE:fir.ptr<f32>]] alloc {
17+
! CHECK: ^bb0(%{{.*}}: ![[X_TYPE]]):
18+
! CHECK: %[[PRIV_ALLOC:.*]] = fir.alloca f32 {bindc_name = "x", {{.*}}}
19+
! CHECK: %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]] {{{.*}}} : (![[PRIV_TYPE:fir.ref<f32>]]) -> ({{.*}})
20+
! CHECK: omp.yield(%[[PRIV_DECL]]#0 : ![[PRIV_TYPE]])
21+
! CHECK: } copy {
22+
! CHECK: ^bb0(%[[ORIG_PTR:.*]]: ![[X_TYPE]], %[[PRIV_REF:.*]]: ![[PRIV_TYPE]]):
23+
! CHECK: %[[ORIG_VAL:.*]] = fir.load %[[ORIG_PTR]] : !fir.ptr<f32>
24+
! CHECK: hlfir.assign %[[ORIG_VAL]] to %[[PRIV_REF]] temporary_lhs : f32, ![[PRIV_TYPE]]
25+
! CHECK: omp.yield(%[[PRIV_REF]] : ![[PRIV_TYPE]])
26+
! CHECK: }
27+
28+
! CHECK: func.func @_QPprivate_common() {
29+
! CHECK: omp.parallel private(@[[X_PRIVATIZER]] %{{.*}}#0 -> %[[PRIV_ARG:.*]] : ![[X_TYPE]]) {
30+
! CHECK: %[[REG_DECL:.*]]:2 = hlfir.declare %[[PRIV_ARG]] {{{.*}}} : (![[X_TYPE]]) -> ({{.*}})
31+
! CHECK: %[[CST:.*]] = arith.constant {{.*}}
32+
! CHECK: hlfir.assign %[[CST]] to %[[REG_DECL]]#0 : {{.*}}
33+
! CHECK: omp.terminator
34+
! CHECK: }
35+
! CHECK: return
36+
! CHECK: }

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2505,7 +2505,11 @@ LogicalResult PrivateClauseOp::verify() {
25052505
<< "Did not expect any values to be yielded.";
25062506
}
25072507

2508-
if (yieldedTypes.size() == 1 && yieldedTypes.front() == symType)
2508+
// TODO How can we check that 2 types are compatible without leaking
2509+
// dialect-specific (e.g. FIR) information? The problem is that in some
2510+
// cases, we would get the input type of the symbol as, e.g. `fir.ptr<i32>`
2511+
// while the allocated private memory is of type `fir.ref<i32>`.
2512+
if (yieldedTypes.size() == 1 /*&& yieldedTypes.front() == symType*/)
25092513
return success();
25102514

25112515
auto error = mlir::emitError(yieldOp.getLoc())

mlir/test/Dialect/OpenMP/invalid.mlir

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,13 +2207,14 @@ func.func @omp_distribute_unconstrained_order() -> () {
22072207
}
22082208
return
22092209
}
2210-
// -----
2211-
omp.private {type = private} @x.privatizer : i32 alloc {
2212-
^bb0(%arg0: i32):
2213-
%0 = arith.constant 0.0 : f32
2214-
// expected-error @below {{Invalid yielded value. Expected type: 'i32', got: 'f32'}}
2215-
omp.yield(%0 : f32)
2216-
}
2210+
2211+
// Disabled for now. See relevant TODO in PrivateClauseOp::verify().
2212+
//omp.private {type = private} @x.privatizer : i32 alloc {
2213+
//^bb0(%arg0: i32):
2214+
// %0 = arith.constant 0.0 : f32
2215+
// // _expected-error_ @below {{Invalid yielded value. Expected type: 'i32', got: 'f32'}}
2216+
// omp.yield(%0 : f32)
2217+
//}
22172218

22182219
// -----
22192220

mlir/test/Target/LLVMIR/openmp-private.mlir

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,3 +234,38 @@ omp.declare_reduction @reducer.part : !llvm.ptr init {
234234
^bb0(%arg0: !llvm.ptr):
235235
omp.yield
236236
}
237+
238+
// -----
239+
240+
llvm.func @_QPequivalence() {
241+
%0 = llvm.mlir.constant(1 : i64) : i64
242+
%1 = llvm.alloca %0 x !llvm.array<4 x i8> : (i64) -> !llvm.ptr
243+
%2 = llvm.mlir.constant(0 : index) : i64
244+
%3 = llvm.getelementptr %1[0, %2] : (!llvm.ptr, i64) -> !llvm.ptr, !llvm.array<4 x i8>
245+
omp.parallel private(@_QFequivalenceEx_firstprivate_ptr_f32 %3 -> %arg0 : !llvm.ptr) {
246+
%4 = llvm.mlir.constant(3.140000e+00 : f32) : f32
247+
llvm.store %4, %arg0 : f32, !llvm.ptr
248+
omp.terminator
249+
}
250+
llvm.return
251+
}
252+
253+
omp.private {type = firstprivate} @_QFequivalenceEx_firstprivate_ptr_f32 : !llvm.ptr alloc {
254+
^bb0(%arg0: !llvm.ptr):
255+
%0 = llvm.mlir.constant(1 : i64) : i64
256+
%1 = llvm.alloca %0 x f32 {bindc_name = "x", pinned} : (i64) -> !llvm.ptr
257+
omp.yield(%1 : !llvm.ptr)
258+
} copy {
259+
^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
260+
%0 = llvm.load %arg0 : !llvm.ptr -> f32
261+
llvm.store %0, %arg1 : f32, !llvm.ptr
262+
omp.yield(%arg1 : !llvm.ptr)
263+
}
264+
265+
// CHECK: define internal void @_QPequivalence..omp_par
266+
// CHECK: %[[PRIV_ALLOC:.*]] = alloca float, i64 1, align 4
267+
// CHECK: %[[HOST_VAL:.*]] = load float, ptr %{{.*}}, align 4
268+
// Test that we initialzie the firstprivate variable.
269+
// CHECK: store float %[[HOST_VAL]], ptr %[[PRIV_ALLOC]], align 4
270+
// Test that we inlined the body of the parallel region.
271+
// CHECK: store float 0x{{.*}}, ptr %[[PRIV_ALLOC]], align 4

0 commit comments

Comments
 (0)