[flang-commits] [flang] [flang][OpenMP][Semantics] Don't allow reduction of derived type components (PR #125480)
Tom Eccles via flang-commits
flang-commits at lists.llvm.org
Mon Feb 3 07:11:37 PST 2025
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/125480
>From 0b6f2e176feda4cee6000ab83ef36512d44df138 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Mon, 3 Feb 2025 11:10:05 +0000
Subject: [PATCH 1/2] [flang][OpenMP][Semantics] Don't allow reduction of
derived type components
Before this patch, reduction of derived type components crashed the
compiler when trying to create the omp.declare_reduction.
In OpenMP 3.1 the standard says "a list item that appears in a reduction
clause must be a named variable of intrinsic type" (page 106). As I
understand it, a derived type component is not a variable.
OpenMP 4.0 added declare reduction, partly so that users could define
their own reductions on derived types. The above wording was removed
from the standard but derived type components were never explicitly
allowed.
OpenMP 5.0 added "A variable that is part of another variable, with
the exception of array elements, cannot appear in17 a reduction
clause".
As we do not currently support derived type components, and these are
only implicitly permitted in OpenMP 4.x, I think it is better to
disallow them in semantics rather than only printing a TODO for openmp
4.x. This is also what gfortran and classic-flang do.
Fixes #125445
---
flang/lib/Semantics/check-omp-structure.cpp | 8 ++++++--
.../Semantics/OpenMP/reduction-derived-component.f90 | 10 ++++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
create mode 100644 flang/test/Semantics/OpenMP/reduction-derived-component.f90
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 035064ecf3a46e..ad36ce2bde0db9 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3224,9 +3224,13 @@ void OmpStructureChecker::CheckReductionObjects(
}
}
+ // Disallowed in standards before 4.0 and in 5.0 and later. Not explicitly
+ // allowed in 4.0. Keep this as an error until/unless structure component
+ // reduction is implemented.
+ // Object cannot be a part of another object (except array elements)
+ CheckStructureComponent(objects, clauseId);
+
if (version >= 50) {
- // Object cannot be a part of another object (except array elements)
- CheckStructureComponent(objects, clauseId);
// If object is an array section or element, the base expression must be
// a language identifier.
for (const parser::OmpObject &object : objects.v) {
diff --git a/flang/test/Semantics/OpenMP/reduction-derived-component.f90 b/flang/test/Semantics/OpenMP/reduction-derived-component.f90
new file mode 100644
index 00000000000000..87e5f1acda159b
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/reduction-derived-component.f90
@@ -0,0 +1,10 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+subroutine intrinsic_reduction
+ type local
+ integer alpha
+ end type local
+ type(local) a
+!ERROR: A variable that is part of another variable cannot appear on the REDUCTION clause
+!$omp parallel reduction(+:a%alpha)
+!$omp end parallel
+end subroutine intrinsic_reduction
>From a4ceaeeda5865f362b9f02cc0ff74e8ebdb9a3cb Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Mon, 3 Feb 2025 14:27:47 +0000
Subject: [PATCH 2/2] Allow derived type component reduction in newer standards
---
flang/lib/Lower/OpenMP/ReductionProcessor.cpp | 3 +++
flang/lib/Semantics/check-omp-structure.cpp | 9 +++++----
.../OpenMP/Todo/reduction-derived-type-component.f90 | 11 +++++++++++
.../Semantics/OpenMP/reduction-derived-component.f90 | 4 +++-
4 files changed, 22 insertions(+), 5 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/Todo/reduction-derived-type-component.f90
diff --git a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
index 4a811f1bdfdf53..871c0ad5e3c08b 100644
--- a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
@@ -557,6 +557,9 @@ void ReductionProcessor::addDeclareReduction(
const semantics::Symbol *symbol = object.sym();
reductionSymbols.push_back(symbol);
mlir::Value symVal = converter.getSymbolAddress(*symbol);
+ if (!symVal)
+ TODO(currentLocation,
+ "Reduction symbol has no definition (e.g. derived type compnent)");
mlir::Type eleType;
auto refType = mlir::dyn_cast_or_null<fir::ReferenceType>(symVal.getType());
if (refType)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index ad36ce2bde0db9..411433d6fc63d6 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3224,11 +3224,12 @@ void OmpStructureChecker::CheckReductionObjects(
}
}
- // Disallowed in standards before 4.0 and in 5.0 and later. Not explicitly
- // allowed in 4.0. Keep this as an error until/unless structure component
- // reduction is implemented.
+ // Disallowed in standards before 4.0 and in 5.0 and 5.1. Not explicitly
+ // allowed in 4.0 and 5.2+ but the clause which forbids this is removed.
// Object cannot be a part of another object (except array elements)
- CheckStructureComponent(objects, clauseId);
+ if (version < 52 || (version >= 40 && version < 50)) {
+ CheckStructureComponent(objects, clauseId);
+ }
if (version >= 50) {
// If object is an array section or element, the base expression must be
diff --git a/flang/test/Lower/OpenMP/Todo/reduction-derived-type-component.f90 b/flang/test/Lower/OpenMP/Todo/reduction-derived-type-component.f90
new file mode 100644
index 00000000000000..53c4b787be64c4
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/reduction-derived-type-component.f90
@@ -0,0 +1,11 @@
+! Has to be v5.2 because in some earlier standards this is a semantic error
+! RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
+subroutine intrinsic_reduction
+ type local
+ integer alpha
+ end type local
+ type(local) a
+! CHECK: not yet implemented: Reduction symbol has no definition
+!$omp parallel reduction(+:a%alpha)
+!$omp end parallel
+end subroutine intrinsic_reduction
diff --git a/flang/test/Semantics/OpenMP/reduction-derived-component.f90 b/flang/test/Semantics/OpenMP/reduction-derived-component.f90
index 87e5f1acda159b..809059ea7b4eee 100644
--- a/flang/test/Semantics/OpenMP/reduction-derived-component.f90
+++ b/flang/test/Semantics/OpenMP/reduction-derived-component.f90
@@ -1,4 +1,6 @@
-! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+! Specifying v5.0 because this is implicitly allowed in some older and newer
+! versions of the standard
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -fopenmp-version=50
subroutine intrinsic_reduction
type local
integer alpha
More information about the flang-commits
mailing list