[flang-commits] [flang] [flang][Semantics][OpenMP] set intrinsic attr for reductions (PR #85114)

[Tom Eccles via flang-commits]

https://github.com/tblah created https://github.com/llvm/llvm-project/pull/85114

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.

>From 4dd1a70e70e602389ab8d8b58309a8820a1c15dd Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Wed, 13 Mar 2024 18:11:00 +0000
Subject: [PATCH] [flang][Semantics][OpenMP] set intrinsic attr for reductions

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.
---
 flang/lib/Semantics/resolve-directives.cpp    |  3 +++
 .../Lower/OpenMP/wsloop-reduction-min2.f90    | 21 +++++++++++++------
 2 files changed, 18 insertions(+), 6 deletions(-)

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