[Mlir-commits] [mlir] [openacc][openmp] Add dialect representation for acc atomic operations (PR #65493)

[Razvan Lupusoru]

razvanlupusoru wrote:

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

https://github.com/llvm/llvm-project/pull/65493