Skip to content

Conversation

tblah
Copy link
Contributor

@tblah tblah commented Apr 30, 2024

Before this patch we crashed lowering intrinsic array reductions.

I think this lost during a rebase. I've added a test to make sure it doesn't break again.

Also fixed the TODO message to be more accurate.

I think this lost during a rebase. I've added a test to make sure it
doesn't break again.

Also fixed the TODO message to be more accurate.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Apr 30, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 30, 2024

@llvm/pr-subscribers-flang-openmp

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

Author: Tom Eccles (tblah)

Changes

Before this patch we crashed lowering intrinsic array reductions.

I think this lost during a rebase. I've added a test to make sure it doesn't break again.

Also fixed the TODO message to be more accurate.


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

2 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/ReductionProcessor.cpp (+3-6)
  • (added) flang/test/Lower/OpenMP/Todo/reduction-array-intrinsic.f90 (+11)
diff --git a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
index 38edd1b4682155..b3f08eb81c799a 100644
--- a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
@@ -811,14 +811,11 @@ void ReductionProcessor::addDeclareReduction(
             *reductionIntrinsic)) {
       ReductionProcessor::ReductionIdentifier redId =
           ReductionProcessor::getReductionType(*reductionIntrinsic);
-      for (const Object &object : objectList) {
-        const Fortran::semantics::Symbol *symbol = object.id();
-        mlir::Value symVal = converter.getSymbolAddress(*symbol);
-        if (auto declOp = symVal.getDefiningOp<hlfir::DeclareOp>())
-          symVal = declOp.getBase();
+      for (mlir::Value symVal : reductionVars) {
         auto redType = mlir::cast<fir::ReferenceType>(symVal.getType());
         if (!redType.getEleTy().isIntOrIndexOrFloat())
-          TODO(currentLocation, "User Defined Reduction on non-trivial type");
+          TODO(currentLocation,
+               "Reduction of some types is not supported for intrinsics");
         decl = createDeclareReduction(
             firOpBuilder,
             getReductionName(getRealName(*reductionIntrinsic).ToString(),
diff --git a/flang/test/Lower/OpenMP/Todo/reduction-array-intrinsic.f90 b/flang/test/Lower/OpenMP/Todo/reduction-array-intrinsic.f90
new file mode 100644
index 00000000000000..49c899238d2a37
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/reduction-array-intrinsic.f90
@@ -0,0 +1,11 @@
+! RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Reduction of some types is not supported for intrinsics
+subroutine max_array_reduction(l, r)
+  integer :: l(:), r(:)
+
+  !$omp parallel reduction(max:l)
+    l = max(l, r)
+  !$omp end parallel
+end subroutine

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tblah tblah merged commit 5ada328 into llvm:main Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants