[flang-commits] [flang] [flang] Propagate volatile on openmp reduction vars (PR #142182)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-fir-hlfir

Author: Asher Mancinelli (ashermancinelli)

<details>
<summary>Changes</summary>

In the openmp reduction clause lowering, reduction variables' reference types were not propagating the volatility of the original variable's type. Add a test and address cases of this in ReductionProcessor.

I'm also looking for other cases of this in the openmp bridge, but haven't found any yet.

---
Full diff: https://github.com/llvm/llvm-project/pull/142182.diff


2 Files Affected:

- (modified) flang/lib/Lower/OpenMP/ReductionProcessor.cpp (+4-2) 
- (added) flang/test/Lower/volatile-openmp1.f90 (+46) 


``````````diff
diff --git a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
index b8aa0deb42dd6..7ef0f2a0ef7c5 100644
--- a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
@@ -389,7 +389,8 @@ static void genBoxCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
   hlfir::LoopNest nest = hlfir::genLoopNest(
       loc, builder, shapeShift.getExtents(), /*isUnordered=*/true);
   builder.setInsertionPointToStart(nest.body);
-  mlir::Type refTy = fir::ReferenceType::get(seqTy.getEleTy());
+  const bool seqIsVolatile = fir::isa_volatile_type(seqTy.getEleTy());
+  mlir::Type refTy = fir::ReferenceType::get(seqTy.getEleTy(), seqIsVolatile);
   auto lhsEleAddr = builder.create<fir::ArrayCoorOp>(
       loc, refTy, lhs, shapeShift, /*slice=*/mlir::Value{},
       nest.oneBasedIndices, /*typeparms=*/mlir::ValueRange{});
@@ -659,7 +660,8 @@ void ReductionProcessor::processReductionArguments(
     assert(fir::isa_ref_type(symVal.getType()) &&
            "reduction input var is passed by reference");
     mlir::Type elementType = fir::dyn_cast_ptrEleTy(symVal.getType());
-    mlir::Type refTy = fir::ReferenceType::get(elementType);
+    const bool symIsVolatile = fir::isa_volatile_type(symVal.getType());
+    mlir::Type refTy = fir::ReferenceType::get(elementType, symIsVolatile);
 
     reductionVars.push_back(
         builder.createConvert(currentLocation, refTy, symVal));
diff --git a/flang/test/Lower/volatile-openmp1.f90 b/flang/test/Lower/volatile-openmp1.f90
new file mode 100644
index 0000000000000..163db953b6b80
--- /dev/null
+++ b/flang/test/Lower/volatile-openmp1.f90
@@ -0,0 +1,46 @@
+! RUN: bbc --strict-fir-volatile-verifier -fopenmp %s -o - | FileCheck %s
+program main
+implicit none
+integer,volatile::a
+integer::n,i
+a=0
+n=1000
+!$omp parallel 
+!$omp do reduction(+:a)
+  do i=1,n
+    a=a+1
+  end do
+!$omp end parallel
+end program
+
+! CHECK-LABEL:   func.func @_QQmain() attributes {fir.bindc_name = "main"} {
+! CHECK:           %[[VAL_0:.*]] = arith.constant 1 : i32
+! CHECK:           %[[VAL_1:.*]] = arith.constant 1000 : i32
+! CHECK:           %[[VAL_2:.*]] = arith.constant 0 : i32
+! CHECK:           %[[VAL_3:.*]] = fir.dummy_scope : !fir.dscope
+! CHECK:           %[[VAL_4:.*]] = fir.alloca i32 {bindc_name = "a", uniq_name = "_QFEa"}
+! CHECK:           %[[VAL_5:.*]] = fir.volatile_cast %[[VAL_4]] : (!fir.ref<i32>) -> !fir.ref<i32, volatile>
+! CHECK:           %[[VAL_6:.*]]:2 = hlfir.declare %[[VAL_5]] {fortran_attrs = #fir.var_attrs<volatile>, uniq_name = "_QFEa"} : (!fir.ref<i32, volatile>) -> (!fir.ref<i32, volatile>, !fir.ref<i32, volatile>)
+! CHECK:           %[[VAL_7:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFEi"}
+! CHECK:           %[[VAL_8:.*]]:2 = hlfir.declare %[[VAL_7]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:           %[[VAL_9:.*]] = fir.alloca i32 {bindc_name = "n", uniq_name = "_QFEn"}
+! CHECK:           %[[VAL_10:.*]]:2 = hlfir.declare %[[VAL_9]] {uniq_name = "_QFEn"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:           hlfir.assign %[[VAL_2]] to %[[VAL_6]]#0 : i32, !fir.ref<i32, volatile>
+! CHECK:           hlfir.assign %[[VAL_1]] to %[[VAL_10]]#0 : i32, !fir.ref<i32>
+! CHECK:           omp.parallel {
+! CHECK:             %[[VAL_11:.*]] = fir.load %[[VAL_10]]#0 : !fir.ref<i32>
+! CHECK:             omp.wsloop private(@_QFEi_private_i32 %[[VAL_8]]#0 -> %[[VAL_12:.*]] : !fir.ref<i32>) reduction(@add_reduction_i32 %[[VAL_6]]#0 -> %[[VAL_13:.*]] : !fir.ref<i32, volatile>) {
+! CHECK:               omp.loop_nest (%[[VAL_14:.*]]) : i32 = (%[[VAL_0]]) to (%[[VAL_11]]) inclusive step (%[[VAL_0]]) {
+! CHECK:                 %[[VAL_15:.*]]:2 = hlfir.declare %[[VAL_12]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:                 %[[VAL_16:.*]]:2 = hlfir.declare %[[VAL_13]] {fortran_attrs = #fir.var_attrs<volatile>, uniq_name = "_QFEa"} : (!fir.ref<i32, volatile>) -> (!fir.ref<i32, volatile>, !fir.ref<i32, volatile>)
+! CHECK:                 hlfir.assign %[[VAL_14]] to %[[VAL_15]]#0 : i32, !fir.ref<i32>
+! CHECK:                 %[[VAL_17:.*]] = fir.load %[[VAL_16]]#0 : !fir.ref<i32, volatile>
+! CHECK:                 %[[VAL_18:.*]] = arith.addi %[[VAL_17]], %[[VAL_0]] : i32
+! CHECK:                 hlfir.assign %[[VAL_18]] to %[[VAL_16]]#0 : i32, !fir.ref<i32, volatile>
+! CHECK:                 omp.yield
+! CHECK:               }
+! CHECK:             }
+! CHECK:             omp.terminator
+! CHECK:           }
+! CHECK:           return
+! CHECK:         }

``````````

</details>


https://github.com/llvm/llvm-project/pull/142182