[flang][Semantics][OpenMP] set intrinsic attr for reductions

[tblah]

Reductions such as min are intrinsic procedures. This distinguishes them from user defined reductions. Previously, the intrinsic attribute was not set when visiting reduction clauses causing them to be missed.

wsloop-reduction-min.f90 (the other min reduction test) worked because it contained "min" used as an intrinsic inside of the body of the reduction. This allowed ResolveNamesVisitor::HandleProcedureName to set the correct attribute on that Symbol.

[llvmbot]

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

@llvm/pr-subscribers-flang-semantics

Author: Tom Eccles (tblah)

Changes

Reductions such as min are intrinsic procedures. This distinguishes them from user defined reductions. Previously, the intrinsic attribute was not set when visiting reduction clauses causing them to be missed.

wsloop-reduction-min.f90 (the other min reduction test) worked because it contained "min" used as an intrinsic inside of the body of the reduction. This allowed ResolveNamesVisitor::HandleProcedureName to set the correct attribute on that Symbol.

---

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

2 Files Affected:

• (modified) flang/lib/Semantics/resolve-directives.cpp (+3)
• (modified) flang/test/Lower/OpenMP/wsloop-reduction-min2.f90 (+15-6)

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 215a3c9060a241..6d58013b87d298 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -487,6 +487,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
       const auto namePair{
           currScope().try_emplace(name->source, Attrs{}, ProcEntityDetails{})};
       auto &newSymbol{*namePair.first->second};
+      if (context_.intrinsics().IsIntrinsic(name->ToString())) {
+        newSymbol.attrs().set(Attr::INTRINSIC);
+      }
       name->symbol = &newSymbol;
     };
     if (const auto *procD{parser::Unwrap<parser::ProcedureDesignator>(opr.u)}) {
diff --git a/flang/test/Lower/OpenMP/wsloop-reduction-min2.f90 b/flang/test/Lower/OpenMP/wsloop-reduction-min2.f90
index 9289973bae2029..86fb067fc48eb7 100644
--- a/flang/test/Lower/OpenMP/wsloop-reduction-min2.f90
+++ b/flang/test/Lower/OpenMP/wsloop-reduction-min2.f90
@@ -17,8 +17,16 @@ program reduce
 
 end program
 
-! TODO: the reduction is not curently lowered correctly. This test is checking
-! that we do not crash and we still produce the same broken IR as before.
+! CHECK-LABEL:   omp.reduction.declare @min_i_32 : i32 init {
+! CHECK:         ^bb0(%[[VAL_0:.*]]: i32):
+! CHECK:           %[[VAL_1:.*]] = arith.constant 2147483647 : i32
+! CHECK:           omp.yield(%[[VAL_1]] : i32)
+
+! CHECK-LABEL:   } combiner {
+! CHECK:         ^bb0(%[[VAL_0:.*]]: i32, %[[VAL_1:.*]]: i32):
+! CHECK:           %[[VAL_2:.*]] = arith.minsi %[[VAL_0]], %[[VAL_1]] : i32
+! CHECK:           omp.yield(%[[VAL_2]] : i32)
+! CHECK:         }
 
 ! CHECK-LABEL:   func.func @_QQmain() attributes {fir.bindc_name = "reduce"} {
 ! CHECK:           %[[VAL_0:.*]] = fir.address_of(@_QFEi) : !fir.ref<i32>
@@ -31,10 +39,11 @@ program reduce
 ! CHECK:             %[[VAL_6:.*]] = arith.constant 0 : i32
 ! CHECK:             %[[VAL_7:.*]] = arith.constant 10 : i32
 ! CHECK:             %[[VAL_8:.*]] = arith.constant 1 : i32
-! CHECK:             omp.wsloop  for  (%[[VAL_9:.*]]) : i32 = (%[[VAL_6]]) to (%[[VAL_7]]) inclusive step (%[[VAL_8]]) {
-! CHECK:               fir.store %[[VAL_9]] to %[[VAL_5]]#1 : !fir.ref<i32>
-! CHECK:               %[[VAL_10:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
-! CHECK:               hlfir.assign %[[VAL_10]] to %[[VAL_3]]#0 : i32, !fir.ref<i32>
+! CHECK:             omp.wsloop reduction(@min_i_32 %[[VAL_3]]#0 -> %[[VAL_9:.*]] : !fir.ref<i32>)  for  (%[[VAL_10:.*]]) : i32 = (%[[VAL_6]]) to (%[[VAL_7]]) inclusive step (%[[VAL_8]]) {
+! CHECK:               fir.store %[[VAL_10]] to %[[VAL_5]]#1 : !fir.ref<i32>
+! CHECK:               %[[VAL_11:.*]]:2 = hlfir.declare %[[VAL_9]] {uniq_name = "_QFEr"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:               %[[VAL_12:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
+! CHECK:               hlfir.assign %[[VAL_12]] to %[[VAL_11]]#0 : i32, !fir.ref<i32>
 ! CHECK:               omp.yield
 ! CHECK:             }
 ! CHECK:             omp.terminator

[kiranchandramohan]

Can you check whether the following renaming works correctly?

module m1
  intrinsic max
end module m1
program main
  use m1, ren=>max
  n=0
!$omp parallel do reduction(ren:n)
  print *, "par"
!$omp end parallel do
end program main

Could you add a fdebug-dump-symbols or a fdebug-unparse-with-symbols test in Semantics/OpenMP?

[tblah]

Thanks for taking a look

Can you check whether the following renaming works correctly?

Yes that works. I added a test for this. The test doesn't need the other changes in this PR to pass

[kiranchandramohan]

LG.