[Frontend][OpenMP] Remove reduction from allowed clauses for target

[kparzysz]

The "reduction" clause is not allowed on the "target" construct.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

The "reduction" clause is not allowed on the "target" construct.

---

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

1 Files Affected:

• (modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (-1)

diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index e91169e8da1aa5..609df6a4c54aa5 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -682,7 +682,6 @@ def OMP_Target : Directive<"target"> {
     VersionedClause<OMPC_FirstPrivate>,
     VersionedClause<OMPC_IsDevicePtr>,
     VersionedClause<OMPC_HasDeviceAddr, 51>,
-    VersionedClause<OMPC_Reduction>,
     VersionedClause<OMPC_InReduction, 50>,
     VersionedClause<OMPC_Allocate>,
     VersionedClause<OMPC_UsesAllocators, 50>,

[kparzysz]

There will be build failures because I haven't changed any tests.

In commit 3f96fe6, Alexey added support for reduction on target, seemingly in response to a change in OpenMP 5.0. The issue is that the 5.0 spec added support for in_reduction, not reduction to target. The reduction clause is not allowed to this day.

Is this some kind of an extension we decided to implement?

[alexey-bataev]

There will be build failures because I haven't changed any tests.

In commit 3f96fe6, Alexey added support for reduction on target, seemingly in response to a change in OpenMP 5.0. The issue is that the 5.0 spec added support for in_reduction, not reduction to target. The reduction clause is not allowed to this day.

Is this some kind of an extension we decided to implement?

I don't remember already, probably some kind of extension to support combined directives codegen (most probably).

[mjklemm]

The current for OpenMP 6.0 also does not allow to have reduction at target and from a semantics perspective it does not really make sense. The first nested construct where this makes sense is teams that has multiple entities to aggregate over.

Unless there's a very good need to keep it, I'd vote for removing it to and not extend OpenMP that way.

[kparzysz]

Is everybody ok with removing it, and removing the parts of the clang tests that use it?

If not, what else should we do? Having it allowed in the .td file will affect flang too. We could implement isAllowedClauseForDirectiveEx in clang that would keep reduction on target, and defer everything else to the existing isAllowedClauseForDirective.

[kiranchandramohan]

I will vote for removing it if it is not permitted in the standard. But it might be worth asking via an RFC in clang or openmp discourse.

[mjklemm]

LGTM, but please update the tests.

[kparzysz]

Created an RFC here: https://discourse.llvm.org/t/removing-reduction-from-allowed-clauses-for-target-in-omp-td/78881.