[PATCH] D118547: [OMPIRBuilder] Add support for atomic compare

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG, minor things to address



================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3490
+           "OMP atomic compare expected a scalar type");
+  });
+
----------------
assertions don't go into LLVM_DEBUG.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3504
+  } else {
+    assert(Op == OMPAtomicCompareOp::MAX || Op == OMPAtomicCompareOp::MIN);
+    assert(X.ElemTy->isIntegerTy() &&
----------------
assertions should have a message


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3523
+        NewOp = Op == OMPAtomicCompareOp::MAX ? AtomicRMWInst::UMin
+                                              : AtomicRMWInst::UMax;
+    } else {
----------------
SMin, SMax


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:3078
+  EXPECT_EQ(ARWM2->getValOperand(), Expr);
+  EXPECT_EQ(ARWM2->getOperation(), AtomicRMWInst::UMax);
+
----------------
Add SMax test too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118547