Removing `reduction` from allowed clauses for `target` in OMP.td

Clang Frontend
1

Background

Commit 3f96fe6 added support for the reduction clause on the target construct. This work was motivated by changes in the OpenMP 5.0 spec, which added in_reduction to target. The commit message contains a mistake: reduction itself was never added to target and is still not in the spec today. The exact reasons for adding reduction as well are now lost in antiquity (see comments in PR90754).

The problem

What remains is that the OMP.td file (that defines OpenMP directives and clauses) claims that reduction in an allowed clause for target. It is a cross-project file, and this has detrimental consequences for other projects (e.g. flang) that implement OpenMP, especially since this is not a deviation that’s easy or desirable to accommodate.

What to do next

I propose that we remove reduction from target in OMP.td, and update clang tests to no longer test that combination.

At the same time, since clang has supported this combination for a few years, there may be applications out there that use it. Should we specifically allow it in clang, and keep the current behavior as an extension, or should we simply remove it as non-compliant with the OpenMP spec?

cc: @alexey-bataev @kiranchandramohan @mjklemm @kkwli @jdoerfert

1 Like
2

I would vote for removing reduction from target since it is not standard conforming.

3

Since this is not standards conforming, it should ideally be removed. I don’t know if the following suggestion aligns with how deprecations are handled in clang , but nonetheless, here is my suggestion -

  • Instead of going cold turkey on this, allow one release of clang to warn that this is going to be deprecated in the next release. And then get rid of it.
  • In the meantime, since flang is starting off on this, do not use this clause in flang and mark it as an error.
4

I second that.

5

I doubt that we really need to issue a deprecation warning. I think the risk of a cold turkey with the community is rather low. The spelling target reduction(...) is rather useless and anyone who was using this with GPUs would have added teams (and maybe loop constructs). So, I’d vote for going the quick and hard route and for simply removing it.

6

I updated the PR90754 with the changes discussed above: remove reduction from clauses allowed on target, and remove tests that exercised this combination.

7

The PR has been merged.