[openacc][openmp] Add dialect representation for acc atomic operations

[razvanlupusoru]

The OpenACC standard specifies an atomic construct in section 2.12 (of 3.3 spec), used to ensure that a specific location is accessed or updated atomically. Four different clauses are allowed: read, write, update, or capture. If no clause appears, it is as if update is used.

The OpenMP specification defines the same clauses for omp atomic. The types of expression and the clauses in the OpenACC spec match the OpenMP spec exactly. The main difference is that the OpenMP specification is a superset - it includes clauses for hint and memory order. It also allows conditional expression statements. But otherwise, the expression definition matches.

Thus, for OpenACC, we refactor and reuse the OpenMP implementation as follows:

• The atomic operations are duplicated in OpenACC dialect. This is preferable so that each language's semantics are precisely represented even if specs have divergence.
• However, since semantics overlap, a common interface between the atomic operations is being added. The semantics for the interfaces are not generic enough to be used outside of OpenACC and OpenMP, and thus new folders were added to hold common pieces of the two dialects.
• The atomic interfaces define common accessors (such as getting x or v) which match the OpenMP and OpenACC specs. It also adds common verifiers intended to be called by each dialect's operation verifier.
• The OpenMP write operation was updated to use x and expr to be consistent with its other operations (that use naming based on spec).

The frontend lowering necessary to generate the dialect can also be reused. This will be done in a follow up change.

Differential Revision: https://reviews.llvm.org/D158768

[vzakhari]

Looks great! Thank you for the changes! I have just minor remarks.

[clementval]

Looks ok for me.

[shraiysh]

This looks great! Thank you for working on this.

[shraiysh]

Looks good to me!

[razvanlupusoru]

@aartbik Since I added new interfaces, several cmake files had to be updated. I attempted to do the same for the bazel build but did not test it. Would you mind looking through the changes? Thank you!

[aartbik]

@aartbik Since I added new interfaces, several cmake files had to be updated. I attempted to do the same for the bazel build but did not test it. Would you mind looking through the changes? Thank you!

It is very kind of you to include the bazel builds too (especially since this is not a requirement for submitting to MLIR), since it helps people integrating changes into bazel-based repos enormously. The changes look good to me (although indeed actually testing them will reveal any lingering issue) and will help integrators a lot!

[grypp]

Hey, I've just seen this PR, I understand the need for a common atomic interface between these two models.

I am little concerned about the OpenACCMPCommon folder is in the dialect folder, but there is no dialect definition here? There is a single interface AtomicInterface, imho atomics can be used beyond just OpenACC/MP.

Would make sense to delete the OpenACCMPCommon folder and move the AtomicInterface to the include/mlir/interfaces?

[razvanlupusoru]

Hey, I've just seen this PR, I understand the need for a common atomic interface between these two models.

I am little concerned about the OpenACCMPCommon folder is in the dialect folder, but there is no dialect definition here? There is a single interface AtomicInterface, imho atomics can be used beyond just OpenACC/MP.

Would make sense to delete the OpenACCMPCommon folder and move the AtomicInterface to the include/mlir/interfaces?

Hi Guray! :)

I agree with your point about it not seeming right being in the dialect folder but with no dialect definition. I did in fact struggle to find the right home for this interface. A couple of factors drove the decision to place it in its current location:

1. I considered the common interface folder - however, there were two aspects that prevented me from adding it there:
   a) The atomic interfaces are currently OpenMP/OpenACC specific. I looked into two aspects to understand whether they could be generalized by looking at LLVM dialect atomics and at C++ std atomics. In both cases, I did not find a natural mapping. For example, atomic read - for both OpenACC and OpenMP the spec is "v = x" and both are specified as l-value expressions. The atomic read from LLVM IR and from std::atomic have more of "value" semantics where the result of read is not to a memory location.
   b) The OpenMP dialect is currently in the process of adopting the same model for the data operations that we introduced with the OpenACC dialect. That said, the two dialects have differences - and at this point it is ideal to not introduce any coupling between the two that would make evolving each dialect more challenging. But, there are obviously natural overlaps - the current idea is that sharing common interfaces will help with this. So the point here is that there are plans to add additional interfaces between the two - and some of these will definitely not be "common" enough to be used outside of these two dialects. And thus these need a home also.
2. Another possibility suggested to me was placing the interfaces in either one of OpenMP or OpenACC folder. There were concerns with this - it gives the wrong impression that one is based on the other.

So, its current home seemed like an reasonable location.

[grypp]

Hey Razvan - long time no chat :) It's great that you are improving the atomic support, nice progress!

I trust your judgment that the interface being OpenMP/OpenACC specific. In that case, would it be a good idea creating include/mlir/interfaces/OpenACCMPCommonInterfaces? You might even need more interfaces between these models, so the folder can be useful. (or anything doesn't involve Dialect folder works for me.) What do you think?

I also gave a thought of placing the interface in either the OpenMP or OpenACC folder. But I completely agree with you that it might not be the best approach.

[razvanlupusoru]

You might even need more interfaces between these models

Agreed! There will definitely be more interfaces! I also imagine not just operation interfaces - but type interfaces too.

In that case, would it be a good idea creating include/mlir/interfaces/OpenACCMPCommonInterfaces

Your suggestion makes sense! My main hesitation is just that it feels eventually we may share more than just interfaces. For example, utilities and ops. I don't want to move things around often - and the current home seems it can be a good home even for future expansion.