[flang-commits] [flang] d507e8b - [flang][OpenMP] Fix firstprivate bug

[Arnamoy Bhattacharyya via flang-commits]

Author: Arnamoy Bhattacharyya
Date: 2022-07-11T09:01:15-04:00
New Revision: d507e8b70e4668f891d5df03f966c154cc4d5370

URL: https://github.com/llvm/llvm-project/commit/d507e8b70e4668f891d5df03f966c154cc4d5370
DIFF: https://github.com/llvm/llvm-project/commit/d507e8b70e4668f891d5df03f966c154cc4d5370.diff

LOG: [flang][OpenMP] Fix firstprivate bug

In case where the bound(s) of a workshare loop use(s) firstprivate var(s), currently, that use is not updated with the created clone.  It still uses the shared variable.  This patch fixes that.

Reviewed By: peixin

Differential Revision: https://reviews.llvm.org/D127137

Added: 
    flang/test/Lower/OpenMP/omp-parallel-wsloop-firstpriv.f90

Modified: 
    flang/lib/Lower/Bridge.cpp

Removed: 
    


################################################################################
diff  --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index aaaf22a7c214..f99eff1c943e 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -472,6 +472,22 @@ class FirConverter : public Fortran::lower::AbstractConverter {
           return fir::substBase(hexv, temp);
         });
 
+    // Replace all uses of the original with the clone/copy,
+    // esepcially for loop bounds (that uses the variable being privatised)
+    // since loop bounds use old values that need to be fixed by using the
+    // new copied value.
+    // Not able to use replaceAllUsesWith() because uses outside
+    // the loop body should not use the clone.
+    mlir::Region &curRegion = getFirOpBuilder().getRegion();
+    mlir::Value oldVal = fir::getBase(hexv);
+    mlir::Value cloneVal = fir::getBase(exv);
+    for (auto &oper : curRegion.getOps()) {
+      for (unsigned int ii = 0; ii < oper.getNumOperands(); ++ii) {
+        if (oper.getOperand(ii) == oldVal) {
+          oper.setOperand(ii, cloneVal);
+        }
+      }
+    }
     return bindIfNewSymbol(sym, exv);
   }
 

diff  --git a/flang/test/Lower/OpenMP/omp-parallel-wsloop-firstpriv.f90 b/flang/test/Lower/OpenMP/omp-parallel-wsloop-firstpriv.f90
new file mode 100644
index 000000000000..79feaa7c3217
--- /dev/null
+++ b/flang/test/Lower/OpenMP/omp-parallel-wsloop-firstpriv.f90
@@ -0,0 +1,63 @@
+! This test checks lowering of OpenMP parallel DO, with the loop bound being
+! a firstprivate variable
+
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+
+! CHECK: func @_QPomp_do_firstprivate(%[[ARG0:.*]]: !fir.ref<i32> {fir.bindc_name = "a"}) 
+subroutine omp_do_firstprivate(a)
+  integer::a
+  integer::n
+  n = a+1
+  !$omp parallel do firstprivate(a)
+  ! CHECK:  omp.parallel {
+  ! CHECK-NEXT: %[[CLONE:.*]] = fir.alloca i32 {bindc_name = "a", pinned
+  ! CHECK-NEXT: %[[LD:.*]] = fir.load %[[ARG0]] : !fir.ref<i32>
+  ! CHECK-NEXT: fir.store %[[LD]] to %[[CLONE]] : !fir.ref<i32>
+  ! CHECK-NEXT: omp.barrier
+  ! CHECK-NEXT: %[[REF:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
+  ! CHECK: %[[LB:.*]] = arith.constant 1 : i32
+  ! CHECK-NEXT: %[[UB:.*]] = fir.load %[[CLONE]] : !fir.ref<i32>
+  ! CHECK-NEXT: %[[STEP:.*]] = arith.constant 1 : i32
+  ! CHECK-NEXT: omp.wsloop   for  (%[[ARG1:.*]]) : i32 = (%[[LB]]) to (%[[UB]]) inclusive step (%[[STEP]])
+  ! CHECK-NEXT: fir.store %[[ARG1]] to %[[REF]] : !fir.ref<i32>
+  ! CHECK-NEXT: fir.call @_QPfoo(%[[REF]], %[[CLONE]]) : (!fir.ref<i32>, !fir.ref<i32>) -> ()
+  ! CHECK-NEXT: omp.yield
+    do i=1, a
+      call foo(i, a)
+    end do
+  !$omp end parallel do
+  !CHECK: fir.call @_QPbar(%[[ARG0]]) : (!fir.ref<i32>) -> ()
+  call bar(a)
+end subroutine omp_do_firstprivate
+
+! CHECK: func @_QPomp_do_firstprivate2(%[[ARG0:.*]]: !fir.ref<i32> {fir.bindc_name = "a"}, %[[ARG1:.*]]: !fir.ref<i32> {fir.bindc_name = "n"}) 
+subroutine omp_do_firstprivate2(a, n)
+  integer::a
+  integer::n
+  n = a+1
+  !$omp parallel do firstprivate(a, n)
+  ! CHECK:  omp.parallel {
+  ! CHECK-NEXT: %[[CLONE:.*]] = fir.alloca i32 {bindc_name = "a", pinned
+  ! CHECK-NEXT: %[[LD:.*]] = fir.load %[[ARG0]] : !fir.ref<i32>
+  ! CHECK-NEXT: fir.store %[[LD]] to %[[CLONE]] : !fir.ref<i32>
+  ! CHECK-NEXT: %[[CLONE1:.*]] = fir.alloca i32 {bindc_name = "n", pinned
+  ! CHECK-NEXT: %[[LD1:.*]] = fir.load %[[ARG1]] : !fir.ref<i32>
+  ! CHECK-NEXT: fir.store %[[LD1]] to %[[CLONE1]] : !fir.ref<i32>
+  ! CHECK-NEXT: omp.barrier
+  ! CHECK-NEXT: %[[REF:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
+
+
+  ! CHECK: %[[LB:.*]] = fir.load %[[CLONE]] : !fir.ref<i32>
+  ! CHECK-NEXT: %[[UB:.*]] = fir.load %[[CLONE1]] : !fir.ref<i32>
+  ! CHECK-NEXT: %[[STEP:.*]] = arith.constant 1 : i32
+  ! CHECK-NEXT: omp.wsloop   for  (%[[ARG2:.*]]) : i32 = (%[[LB]]) to (%[[UB]]) inclusive step (%[[STEP]])
+  ! CHECK-NEXT: fir.store %[[ARG2]] to %[[REF]] : !fir.ref<i32>
+  ! CHECK-NEXT: fir.call @_QPfoo(%[[REF]], %[[CLONE]]) : (!fir.ref<i32>, !fir.ref<i32>) -> ()
+  ! CHECK-NEXT: omp.yield
+    do i= a, n
+      call foo(i, a)
+    end do
+  !$omp end parallel do
+  !CHECK: fir.call @_QPbar(%[[ARG1]]) : (!fir.ref<i32>) -> ()
+  call bar(n)
+end subroutine omp_do_firstprivate2