Skip to content

Commit bba50a4

Browse files
committed
[flang] Do not re-localize loop ivs when nested inside blocks
Consider the following example: ```fortran implicit none integer :: i, j do concurrent (i=1:10) local(j) block do j=1,20 end do end block end do ``` Without the fix introduced in this PR, the compiler would "re-localize" the `j` variable inside the `fir.do_concurrent` loop: ``` fir.do_concurrent { %7 = fir.alloca i32 {bindc_name = "j"} %8:2 = hlfir.declare %7 {uniq_name = "_QFloop_in_nested_blockEj"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) ... fir.do_concurrent.loop (%arg0) = (%5) to (%6) step (%c1) local(@_QFloop_in_nested_blockEj_private_i32 %4#0 -> %arg1 : !fir.ref<i32>) { %12:2 = hlfir.declare %arg1 {uniq_name = "_QFloop_in_nested_blockEj"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) ... %17:2 = fir.do_loop %arg2 = %14 to %15 step %c1_1 iter_args(%arg3 = %16) -> (index, i32) { fir.store %arg3 to %8#0 : !fir.ref<i32> ... } } } ``` This happened because we did a shallow look-up of `j` and since the loop is nested inside a `block`, the look-up failed and we re-created a local allocation for `j` inside the parent `fir.do_concurrent` loop. This means that we ended up not using the actual localized symbol which is passed as a region argument to the `fir.do_concurrent.loop` op. In case of `j`, we do not need to do a shallow look-up. The shallow look-up is only needed if a symbol is an OpenMP private one or an iteration variable of a `do concurrent` loop. Neither of which applies to `j`. With the fix, `j` is properly resolved to the `local` region argument: ``` fir.do_concurrent { ... fir.do_concurrent.loop (%arg0) = (%5) to (%6) step (%c1) local(@_QFloop_in_nested_blockEj_private_i32 %4#0 -> %arg1 : !fir.ref<i32>) { ... %10:2 = hlfir.declare %arg1 {uniq_name = "_QFloop_in_nested_blockEj"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) ... %15:2 = fir.do_loop %arg2 = %12 to %13 step %c1_1 iter_args(%arg3 = %14) -> (index, i32) { fir.store %arg3 to %10#0 : !fir.ref<i32> ... } } } ```
1 parent ca44e11 commit bba50a4

File tree

2 files changed

+43
-15
lines changed

2 files changed

+43
-15
lines changed

flang/lib/Lower/Bridge.cpp

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,21 +1400,23 @@ class FirConverter : public Fortran::lower::AbstractConverter {
14001400
mlir::Value genLoopVariableAddress(mlir::Location loc,
14011401
const Fortran::semantics::Symbol &sym,
14021402
bool isUnordered) {
1403-
if (isUnordered || sym.has<Fortran::semantics::HostAssocDetails>() ||
1404-
sym.has<Fortran::semantics::UseDetails>()) {
1405-
if (!shallowLookupSymbol(sym) &&
1406-
!GetSymbolDSA(sym).test(
1407-
Fortran::semantics::Symbol::Flag::OmpShared)) {
1408-
// Do concurrent loop variables are not mapped yet since they are local
1409-
// to the Do concurrent scope (same for OpenMP loops).
1410-
mlir::OpBuilder::InsertPoint insPt = builder->saveInsertionPoint();
1411-
builder->setInsertionPointToStart(builder->getAllocaBlock());
1412-
mlir::Type tempTy = genType(sym);
1413-
mlir::Value temp =
1414-
builder->createTemporaryAlloc(loc, tempTy, toStringRef(sym.name()));
1415-
bindIfNewSymbol(sym, temp);
1416-
builder->restoreInsertionPoint(insPt);
1417-
}
1403+
if (!shallowLookupSymbol(sym) &&
1404+
(isUnordered ||
1405+
GetSymbolDSA(sym).test(Fortran::semantics::Symbol::Flag::OmpPrivate) ||
1406+
GetSymbolDSA(sym).test(
1407+
Fortran::semantics::Symbol::Flag::OmpFirstPrivate) ||
1408+
GetSymbolDSA(sym).test(
1409+
Fortran::semantics::Symbol::Flag::OmpLastPrivate) ||
1410+
GetSymbolDSA(sym).test(Fortran::semantics::Symbol::Flag::OmpLinear))) {
1411+
// Do concurrent loop variables are not mapped yet since they are
1412+
// local to the Do concurrent scope (same for OpenMP loops).
1413+
mlir::OpBuilder::InsertPoint insPt = builder->saveInsertionPoint();
1414+
builder->setInsertionPointToStart(builder->getAllocaBlock());
1415+
mlir::Type tempTy = genType(sym);
1416+
mlir::Value temp =
1417+
builder->createTemporaryAlloc(loc, tempTy, toStringRef(sym.name()));
1418+
bindIfNewSymbol(sym, temp);
1419+
builder->restoreInsertionPoint(insPt);
14181420
}
14191421
auto entry = lookupSymbol(sym);
14201422
(void)entry;
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
! RUN: %flang_fc1 -emit-hlfir -mmlir --enable-delayed-privatization-staging=true -o - %s | FileCheck %s
2+
3+
subroutine loop_in_nested_block
4+
implicit none
5+
integer :: i, j
6+
7+
do concurrent (i=1:10) local(j)
8+
block
9+
do j=1,20
10+
end do
11+
end block
12+
end do
13+
end subroutine
14+
15+
! CHECK-LABEL: func.func @_QPloop_in_nested_block() {
16+
! CHECK: %[[OUTER_J_DECL:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "{{.*}}Ej"}
17+
! CHECK: fir.do_concurrent {
18+
! CHECK: fir.do_concurrent.loop {{.*}} local(@{{.*}} %[[OUTER_J_DECL]]#0 -> %[[LOCAL_J_ARG:.*]] : !fir.ref<i32>) {
19+
! CHECK: %[[LOCAL_J_DECL:.*]]:2 = hlfir.declare %[[LOCAL_J_ARG]]
20+
! CHECK: fir.do_loop {{.*}} iter_args(%[[NESTED_LOOP_ARG:.*]] = {{.*}}) {
21+
! CHECK: fir.store %[[NESTED_LOOP_ARG]] to %[[LOCAL_J_DECL]]#0
22+
! CHECK: }
23+
! CHECK: }
24+
! CHECK: }
25+
! CHECK: }
26+

0 commit comments

Comments
 (0)