[flang][OpenMP][Semantics] Don't allow reduction of derived type components

[tblah]

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".

All standard versions also require the reduction argument to be "definable", which roughly means that it is a variable. A
derived type component is more like an expression.

Fixes #125445

[llvmbot]

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

@llvm/pr-subscribers-flang-semantics

Author: Tom Eccles (tblah)

Changes

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

---

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

2 Files Affected:

• (modified) flang/lib/Semantics/check-omp-structure.cpp (+6-2)
• (added) flang/test/Semantics/OpenMP/reduction-derived-component.f90 (+10)

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

[kiranchandramohan]

This is OK up to OpenMP 5.1.
A variable that is part of another variable, with the exception of array elements cannotappear in a reduction clause.

OpenMP 5.2 onwards has the following:
A list item that appears in a reduction clause must be definable.

Is it better to emit the semantics error for 5.1 and before and the lowering error for 5.2 and later standards?

[tblah]

This is OK up to OpenMP 5.1. A variable that is part of another variable, with the exception of array elements cannotappear in a reduction clause.

OpenMP 5.2 onwards has the following: A list item that appears in a reduction clause must be definable.

Is it better to emit the semantics error for 5.1 and before and the lowering error for 5.2 and later standards?

Well spotted. I will do that.

[tblah]

@kiranchandramohan I have added the TODO in lowering, but from the lowering error I wonder if we should really consider a derived type component as definable.

That wording is in all versions of the standard (IIRC). I previously assumed that it wasn't relevant here because CheckReductionObjects() already unconditionally calls CheckDefinableObjects().

The error in lowering is because that object cannot be looked up in the symbol table because a component of a derived type isn't a symbol with a definition. I think instance%component is an expression and so is not definable.

The wording in the Fortran standard is in section 3.47 of f2023. It says a definable object is capable of definition and permitted to become defined. Both of those conditions seem to require it to be a data object (3.42):

constant, variable, or subobject of a constant

I think a derived type member is not constant and not a variable (if it were a variable we would be able to look it up in the symbol table).

[kiranchandramohan]

@kiranchandramohan I have added the TODO in lowering, but from the lowering error I wonder if we should really consider a derived type component as definable.

That wording is in all versions of the standard (IIRC). I previously assumed that it wasn't relevant here because CheckReductionObjects() already unconditionally calls CheckDefinableObjects().

The error in lowering is because that object cannot be looked up in the symbol table because a component of a derived type isn't a symbol with a definition. I think instance%component is an expression and so is not definable.

How does it work for array elements? Isn't there a similar issue?

The wording in the Fortran standard is in section 3.47 of f2023. It says a definable object is capable of definition and permitted to become defined. Both of those conditions seem to require it to be a data object (3.42):

constant, variable, or subobject of a constant

I think a derived type member is not constant and not a variable (if it were a variable we would be able to look it up in the symbol table).

Does this exclude array elements also?

[tblah]

How does it work for array elements? Isn't there a similar issue?

It seems we just do reduction on the whole array, ignoring the indexing.

Does this exclude array elements also?

I imagine it should do. gfortran and classic-flang don't allow it.

So does this sound okay to you:

1. I change this patch to deny derived type components for all standard versions
2. I do a followup patch denying array indexing

[kiranchandramohan]

How does it work for array elements? Isn't there a similar issue?

It seems we just do reduction on the whole array, ignoring the indexing.

The following program seemed to work correctly with ifx.
I see that flang currently generates a combiner that does the reduction on the whole array but the final answer it computes is also correct.

program mn
  integer :: i
  integer :: arr(10)
  arr = 0
  !$omp parallel do reduction(+:arr(3))
  do i=1,10
    arr(3) = arr(3) + i
  end do
  !$omp end parallel do
  print *, arr
end program

Does this exclude array elements also?

I imagine it should do. gfortran and classic-flang don't allow it.

So does this sound okay to you:

1. I change this patch to deny derived type components for all standard versions
2. I do a followup patch denying array indexing

I am not 100% sure about this since previous versions explicitly allowed array elements. It might be worth asking a question to the OpenMP committee.

[kkwli]

I am not 100% sure about this since previous versions explicitly allowed array elements. It might be worth asking a question to the OpenMP committee.

OpenMP 6.0 clarifies that array elements are allowed on the clause.

"The list items that appear in a reduction clause or an induction clause may include array sections and array elements." (247:3-4)

[kiranchandramohan]

I am not 100% sure about this since previous versions explicitly allowed array elements. It might be worth asking a question to the OpenMP committee.

OpenMP 6.0 clarifies that array elements are allowed on the clause.

"The list items that appear in a reduction clause or an induction clause may include array sections and array elements." (247:3-4)

Thanks @kkwli. Would you know why derived type members are not allowed?

[kkwli]

I am not 100% sure about this since previous versions explicitly allowed array elements. It might be worth asking a question to the OpenMP committee.

OpenMP 6.0 clarifies that array elements are allowed on the clause.
"The list items that appear in a reduction clause or an induction clause may include array sections and array elements." (247:3-4)

Thanks @kkwli. Would you know why derived type members are not allowed?

For the array elements, one can treat it as an array section with stride 1.

For derived type components, the spec does not have the semantic of subobject privatization yet. For example, if some components of a derived type is privatized, are other components in the same derived type shared? What if the whole derived type is accessed? We haven't figured out the details yet. That is one of the reasons why subobjects are disallowed on most of the data-sharing attribute clauses.

[tblah]

I have updated this to deny reduction of derived type components for all standard versions and updated the comment and description explaining the reasoning.

I have made no change to how array elements are handled. They will continue to be allowed.

[kiranchandramohan]

For derived type components, the spec does not have the semantic of subobject privatization yet. For example, if some components of a derived type is privatized, are other components in the same derived type shared? What if the whole derived type is accessed? We haven't figured out the details yet. That is one of the reasons why subobjects are disallowed on most of the data-sharing attribute clauses.

Thanks @kkwli. Aren't these the same questions for arrays as well, Like, are the other elements of the same array shared, what if the whole array accessed?

[kiranchandramohan]

LG. Thanks.