[Mlir-commits] [flang] [llvm] [mlir] [OpenMPIRBuilder] Emit __atomic_load and __atomic_compare_exchange libcalls for complex types in atomic update (PR #92364)

[llvmlistbot at llvm.org]

NimishMishra wrote:

Hi @kiranchandramohan, thanks for the review. 

> 
> 1. Is the following sentence in the summary still correct or is the code from LLVM AtomicExpand?

Updated the description.
 
> 2. The test removed from Todo is an atomic read, why was it not added back?

I thought it would be good to deal with it in another PR since it deals with atomic read. I have already created the PR in my local fork: https://github.com/NimishMishra/f18-llvm-project/commit/b61d1837b95e7f5dd431553f543f25e598100b0c; I will push it for review as soon as this PR lands (will add mlir tests before pushing).

We would also need a similar handling for atomic write, will push a PR for that too.

> 3. The code change in OpenMPToLLVMIRTranslation.cpp is the Capture Code generation. There is no test for this. Also cand this change be shared with the Update code generation?

   Added tests.

   Sharing code is possible, but it requires changing some interfaces. Reason being that `convertOmpAtomicCapture` directly calls `createAtomicCapture` in the IRBuilder, so it needs all details for the atomic update operation (which `convertOmpAtomicUpdate` cannot provide in its current function signature).

   One way could be to deprecate `createAtomicCapture` and emit the two operations directly in `OpenMPToLLVMIRTranslation.cpp`. Suppose we need to emit (update, read), then invoke `convertOmpAtomicUpdate` and `convertOmpAtomicRead`, which in turn invoke `createAtomicUpdate` and `createAtomicRead` in the IRBuilder respectively.

   Another way could be `convertOmpAtomicCapture` invokes `convertOmpAtomicUpdate` and passes a bunch of pointers such that `convertOmpAtomicUpdate` sends back all necessary details for the update operation. Then `convertOmpAtomicCapture` can invoke `createAtomicCapture` in the IRBuilder normally. This feels a bit cumbersome though.

> 4. We need tests in the OpenMP IRBuilder and in MLIR for the changes.

I've added some tests. Do they look okay?

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