[PATCH] D99024: [OpenMP][OMPIRBuilder] Adding support for `omp atomic`

[Fady Ghanim via Phabricator via llvm-commits]

fghanim added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1936
+
+  if (AK == Read) {
+    if (AO == AtomicOrdering::Acquire || AO == AtomicOrdering::AcquireRelease ||
----------------
Meinersbur wrote:
> This if-chain could be done more elegantly with a switch.
Either:
1- do nested if-else
2- nested switch case
3- a mix of both.

4- use the magic of logical shifts & bit-wise and/or.s to do this, and get rid of all the messy cfg

switched from 1 to 3, more than happy to do 4


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2082-2088
+  case AtomicRMWInst::Xchg:
+  case AtomicRMWInst::FAdd:
+  case AtomicRMWInst::FSub:
+  case AtomicRMWInst::BAD_BINOP:
+  case AtomicRMWInst::Max:
+  case AtomicRMWInst::Min:
+  default:
----------------
Meinersbur wrote:
> With a default case, no need to enumerate all cases.

I intentionally listed all cases. If you want, i'll remove `default` which I added to future proof the code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99024/new/

https://reviews.llvm.org/D99024