[OpenMP][OMPIRBuilder] Adding support for `omp atomic`

[fghanim]

Summary

This patch adds support for generating omp atomic for all different atomic clauses

[Meinersbur]

Could you please address the numerous clang-format/clang-tidy warnings?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2235–2236

2243–2244

Please remove all code in comments; The TODO should explain what there is todo.

2246

This if-chain could be done more elegantly with a switch.

2283

[nit] Unnecessary newline

2287

[style] The LLVM source style does not use almost always auto.

2302

[style] Please start sentences with a capital letter.

2392–2398

With a default case, no need to enumerate all cases.

[fghanim]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2246

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

2392–2398

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

[fghanim]

• fixed clang-tidy errors
• addressed reviewer comments

[fghanim]

• more clang-tidy fixes

[Meinersbur]

You can avoid the "clang-format not found in user's PATH; not linting file." warning using arc diff --nolint. It asks for a justification, it accepts every non-empty string, including a single space. Please consider fixing the other warnings.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2232

Consider making such helpers static functions instead of priviate methods, so they don't need to appear in the public header file.

2246

It is fine as-is, just the Capture case might justify a nested switch.

Not sure whether approach 4 would make it any better.

2392–2398

With a default case, the compiler does not warn about unhandled cases anymore. If you want both, you could do:

[...]
case AtomicRMWInst::Min:
  llvm_unreachable("Unsupported atomic update operation");
}
llvm_unreacheable("Unhandled atomic update operation");

Personally I think it just adds clutter lines. In case of some parts of clang's OpenMP implementation it fills entire screens of unsupported cases and there is no chance I miss handling the case during testing due to the llvm_unreachable.

Currently, return nullptr; is unreachable code.

2434–2438

Triple slashes are doxygen comments, which come before the function. If not a doxygen documentation, please use standard // comments.

2439

[style] No almost-always-auto in LLVM style

2471–2474

Debugging leftover?

[SouraVX]

Thanks! @fghanim for the patch :)

I noticed following piece of code getting used in createAtomicRead, createAtomicWrite, createAtomicCapture, createAtomicUpdate.

if (!updateToLocation(Loc))
    return Loc.IP;
Type *XTy = X.Var->getType();
  assert(XTy->isPointerTy() && "OMP Atomic expects a pointer to target memory");
  Type *XElemTy = XTy->getPointerElementType();
  assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
          XElemTy->isPointerTy()) &&
         "OMP atomic read expected a scalar type");

Was wondering if this could be simplified or hoisted out in a tiny innocuous helper function ?

Also right now we have these 4 public interfaces with external front-end's. Can we simplify this into 2 public interfaces as createAtomicReadOrWrite and createAtomicCaptureOrUpdate ?
The rationale behind this proposition is that, these functions share a lot common code(at expense of 1 or 2 extra argument needed to handle the other case).
Can we do this ? Or doing such simplification will render the code more unreadable/complicated ?
Please Let me know WDYT ?
+ @AMDChirag

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

912

Can we use a better/descriptive name here ? Maybe TargetAtomicPtr ?
Other references should also benefit from descriptive name.

1015

This could also benefit from descriptive naming ?

[fghanim]

In D99024#2667320, @SouraVX wrote:

Thanks! @fghanim for the patch :)

Sure! :)

I noticed following piece of code getting used in createAtomicRead, createAtomicWrite, createAtomicCapture, createAtomicUpdate.

if (!updateToLocation(Loc))
    return Loc.IP;
Type *XTy = X.Var->getType();
  assert(XTy->isPointerTy() && "OMP Atomic expects a pointer to target memory");
  Type *XElemTy = XTy->getPointerElementType();
  assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
          XElemTy->isPointerTy()) &&
         "OMP atomic read expected a scalar type");

Was wondering if this could be simplified or hoisted out in a tiny innocuous helper function ?

The return returns from createOMPAtomicXXX calls, and each of XTy and XElemTy are used inside the function. Also, We are checking different kinds of errors between different atomic clauses, and issuing different error assert messages based on the error, .

Also right now we have these 4 public interfaces with external front-end's. Can we simplify this into 2 public interfaces as createAtomicReadOrWrite and createAtomicCaptureOrUpdate ?
The rationale behind this proposition is that, these functions share a lot common code(at expense of 1 or 2 extra argument needed to handle the other case).
Can we do this ? Or doing such simplification will render the code more unreadable/complicated ?
Please Let me know WDYT ?

I don't think we are simplifying things by doing that, because it will cause unnecessary confusion. two parts:

• Regarding atomic read and atomic write, there is nothing in common between the two (other than the initial checks). While at first glance, they may look similar, they are not.
• Regarding atomic update and atomic capture, I already implemented the parts that are common between the two separately into emitAtomicUpdate

In general, it is better for someone handling an atomic read, to just call createOMPAtomicRead, than call something that is also doing writes. more intuitive.

+ @AMDChirag

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

912

These names are picked to match the naming used in the OMP standard documents for atomic:
https://www.openmp.org/spec-html/5.0/openmpsu95.html

1015

These names are picked to match the naming used in the OMP standard documents for atomic:
https://www.openmp.org/spec-html/5.0/openmpsu95.html

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2232

I call emitFlush() which is private

2392–2398

The issue here is that, these cases are either not supported in omp (min,max,..), or should be handled by the cmpExch (fadd, fsub,exch). I worry if i don't explicitly list them as unreachable somebody might try to implement them.

2471–2474

Yes. Thanks for catching that. removed

[fghanim]

• addressing more reviewer comments
• Fixing nits + clang-format errors

[fghanim]

fixing some clang-tidy issues

[fghanim]

ping

[tianshilei1992]

Thanks for the patch! FYI, I'm working on the FE part of compare and compare capture.

[Meinersbur]

Sorry for the delay.

Consider starting all sentences in comments with capital letters.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

885–887

Could you insert empty lines, at least separate description from the \param list.

Consider surrounding code with \code \endcode (https://www.doxygen.nl/manual/commands.html#cmdcode)

: expr is any legal operation is not a comment in C/C++. Did you mean to capitalize expr?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2282

Capitalize "TODO" to make them stand out (some IDEs recognize these)

2315

There is no test coverage for this non-integer case.

2321

Use a common convention for naming IR registers.

2415

Please completely remove commented code.

2462

"X.Temp" could be more descriptive.

Also, they should conform to the lowercase convention

2476

Use lower case for register names

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

2187

Please add

EXPECT_FALSE(verifyModule(*M, &errs()));

to each tests case to verify its wellformedness.

2191–2192

[style] Only use auto in selected circumstances.

[fghanim]

• Rebasing
• Adding test cases
• Addressing reviewers comments

[fghanim]

In D99024#2694893, @tianshilei1992 wrote:

Thanks for the patch! FYI, I'm working on the FE part of compare and compare capture.

Sure! :)
Let me know if I can help.

In D99024#2695672, @Meinersbur wrote:

Sorry for the delay.

No worries. Thanks for the comments.

[fghanim]

• clang-tidy + clang-format fixes

[Meinersbur]

Functionally, I think the patch is fine. There are some formatting issues left.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

921

[nit] indention

932–933

941–942

Start sentences with capital letters.

Add empty line between description and \param and \return

947

Use a doxygen /// comment?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2392–2398

I like the "OpenMP atomic does not support ..." approach to clarify it is by design.

2551

"make monotonic ordering we have different"
incomplete sentence?

2553

[style] The LLVM source style does not use almost always auto.

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

2295–2306

Please complete remove commented code.

[fghanim]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2551

rephrased the comment

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

2295–2306

leftover code removed.

[fghanim]

• Addressing reviewer's comments

[Meinersbur]

LGTM, thank you.

[fghanim]

Thanks for the review :)

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

private:
 
   /// Returns corresponding lock object for the specified critical region
   /// name. If the lock object does not exist it is created, otherwise the
   /// reference to the existing copy is returned.
   /// \param CriticalName Name of the critical region.
   ///
   Value *getOMPCriticalRegionLock(StringRef CriticalName);
 
+  /// Callback type for Atomic Expression update
+  /// ex:
+  /// \code{.cpp}
+  /// unsigned x = 0;
+  /// #pragma omp atomic update
+  /// x = Expr(x_old);  //Expr() is any legal operation
+  /// \endcode
+  ///
+  /// \param XOld the value of the atomic memory address to use for update
+  /// \param IRB reference to the IRBuilder to use
+  ///
+  /// \returns Value to update X to.
+  using AtomicUpdateCallbackTy =
+      const function_ref<Value *(Value *XOld, IRBuilder<> &IRB)>;
+
+private:
+  enum AtomicKind { Read, Write, Update, Capture };
+
+  /// Determine whether to emit flush or not
+  ///
+  /// \param Loc    The insert and source location description.
+  /// \param AO     The required atomic ordering
+  /// \param AK     The OpenMP atomic operation kind used.
+  ///
+  /// \returns    wether a flush was emitted or not
+  bool checkAndEmitFlushAfterAtomic(const LocationDescription &Loc,
+                                    AtomicOrdering AO, AtomicKind AK);
+
+  /// Emit atomic update for constructs: X = X BinOp Expr ,or X = Expr BinOp X
+  /// For complex Operations: X = UpdateOp(X) => CmpExch X, old_X, UpdateOp(X)
+  /// Only Scalar data types.
+  ///
+  /// \param AllocIP    Instruction to create AllocaInst before.
+  /// \param X          The target atomic pointer to be updated
+  /// \param Expr       The value to update X with.
+  /// \param AO         Atomic ordering of the generated atomic
+  ///                   instructions.
+  /// \param RMWOp      The binary operation used for update. If
+  ///                   operation is not supported by atomicRMW,
+  ///                   or belong to {FADD, FSUB, BAD_BINOP}.
+  ///                   Then a `cmpExch` based  atomic will be generated.
+  /// \param UpdateOp   Code generator for complex expressions that cannot be
+  ///                   expressed through atomicrmw instruction.
+  /// \param VolatileX       true if \a X volatile?
+  /// \param IsXLHSInRHSPart true if \a X is Left H.S. in Right H.S. part of
+  ///                        the update expression, false otherwise.
+  ///                        (e.g. true for X = X BinOp Expr)
+  ///
+  /// \returns A pair of the old value of X before the update, and the value
+  ///          used for the update.
+  std::pair<Value *, Value *> emitAtomicUpdate(Instruction *AllocIP, Value *X,
+                                               Value *Expr, AtomicOrdering AO,
+                                               AtomicRMWInst::BinOp RMWOp,
+                                               AtomicUpdateCallbackTy &UpdateOp,
+                                               bool VolatileX,
+                                               bool IsXLHSInRHSPart);
+
+  /// Emit the binary op. described by \p RMWOp, using \p Src1 and \p Src2 .
+  ///
+  /// \Return The instruction
+  Value *emitRMWOpAsInstruction(Value *Src1, Value *Src2,
+                                AtomicRMWInst::BinOp RMWOp);
+
+public:
+  /// a struct to pack relevant information while generating atomic Ops
+  struct AtomicOpValue {
+    Value *Var = nullptr;
+    bool IsSigned = false;
+    bool IsVolatile = false;
+  };
+
+  /// Emit atomic Read for : V = X --- Only Scalar data types.
+  ///
+  /// \param Loc    The insert and source location description.
+  /// \param X      The target pointer to be atomically read
+  /// \param V      Memory address where to store atomically read
+  ///               value
+  /// \param AO     Atomic ordering of the generated atomic
+  ///               instructions.
+  ///
+  /// \return Insertion point after generated atomic read IR.
+  InsertPointTy createAtomicRead(const LocationDescription &Loc,
+                                 AtomicOpValue &X, AtomicOpValue &V,
+                                 AtomicOrdering AO);
+
+  /// Emit atomic write for : X = Expr --- Only Scalar data types.
+  ///
+  /// \param Loc    The insert and source location description.
+  /// \param X      The target pointer to be atomically written to
+  /// \param Expr   The value to store.
+  /// \param AO     Atomic ordering of the generated atomic
+  ///               instructions.
+  ///
+  /// \return Insertion point after generated atomic Write IR.
+  InsertPointTy createAtomicWrite(const LocationDescription &Loc,
+                                  AtomicOpValue &X, Value *Expr,
+                                  AtomicOrdering AO);
+
+  /// Emit atomic update for constructs: X = X BinOp Expr ,or X = Expr BinOp X
+  /// For complex Operations: X = UpdateOp(X) => CmpExch X, old_X, UpdateOp(X)
+  /// Only Scalar data types.
+  ///
+  /// \param Loc      The insert and source location description.
+  /// \param AllocIP  Instruction to create AllocaInst before.
+  /// \param X        The target atomic pointer to be updated
+  /// \param Expr     The value to update X with.
+  /// \param AO       Atomic ordering of the generated atomic instructions.
+  /// \param RMWOp    The binary operation used for update. If operation
+  ///                 is  not supported by atomicRMW, or belong to
+  ///                 {FADD, FSUB, BAD_BINOP}. Then a `cmpExch` based
+  ///                 atomic will be generated.
+  /// \param UpdateOp   Code generator for complex expressions that cannot be
+  ///                   expressed through atomicrmw instruction.
+  /// \param IsXLHSInRHSPart true if \a X is Left H.S. in Right H.S. part of
+  ///                        the update expression, false otherwise.
+  ///                        (e.g. true for X = X BinOp Expr)
+  ///
+  /// \return Insertion point after generated atomic update IR.
+  InsertPointTy createAtomicUpdate(const LocationDescription &Loc,
+                                   Instruction *AllocIP, AtomicOpValue &X,
+                                   Value *Expr, AtomicOrdering AO,
+                                   AtomicRMWInst::BinOp RMWOp,
+                                   AtomicUpdateCallbackTy &UpdateOp,
+                                   bool IsXLHSInRHSPart);
+
+  /// Emit atomic update for constructs: --- Only Scalar data types
+  /// V = X; X = X BinOp Expr ,
+  /// X = X BinOp Expr; V = X,
+  /// V = X; X = Expr BinOp X,
+  /// X = Expr BinOp X; V = X,
+  /// V = X; X = UpdateOp(X),
+  /// X = UpdateOp(X); V = X,
+  ///
+  /// \param Loc        The insert and source location description.
+  /// \param AllocIP    Instruction to create AllocaInst before.
+  /// \param X          The target atomic pointer to be updated
+  /// \param V          Memory address where to store captured value
+  /// \param Expr       The value to update X with.
+  /// \param AO         Atomic ordering of the generated atomic instructions
+  /// \param RMWOp      The binary operation used for update. If
+  ///                   operation is not supported by atomicRMW, or belong to
+  ///                   {FADD, FSUB, BAD_BINOP}. Then a cmpExch based
+  ///                   atomic will be generated.
+  /// \param UpdateOp   Code generator for complex expressions that cannot be
+  ///                   expressed through atomicrmw instruction.
+  /// \param UpdateExpr true if X is an in place update of the form
+  ///                   X = X BinOp Expr or X = Expr BinOp X
+  /// \param IsXLHSInRHSPart true if X is Left H.S. in Right H.S. part of the
+  ///                        update expression, false otherwise.
+  ///                        (e.g. true for X = X BinOp Expr)
+  /// \param IsPostfixUpdate true if original value of 'x' must be stored in
+  ///                        'v', not an updated one.
+  ///
+  /// \return Insertion point after generated atomic capture IR.
+  InsertPointTy
+  createAtomicCapture(const LocationDescription &Loc, Instruction *AllocIP,
+                      AtomicOpValue &X, AtomicOpValue &V, Value *Expr,
+                      AtomicOrdering AO, AtomicRMWInst::BinOp RMWOp,
+                      AtomicUpdateCallbackTy &UpdateOp, bool UpdateExpr,
+                      bool IsPostfixUpdate, bool IsXLHSInRHSPart);
+
   /// Create the control flow structure of a canonical OpenMP loop.
   ///
   /// The emitted loop will be disconnected, i.e. no edge to the loop's
   /// preheader and no terminator in the AfterBB. The OpenMPIRBuilder's
   /// IRBuilder location is not preserved.
   ///
   /// \param DL        DebugLoc used for the instructions in the skeleton.
   /// \param TripCount Value to be used for the trip count.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

 }
 
 Value *OpenMPIRBuilder::getOMPCriticalRegionLock(StringRef CriticalName) {
   std::string Prefix = Twine("gomp_critical_user_", CriticalName).str();
   std::string Name = getNameWithSeparators({Prefix, "var"}, ".", ".");
   return getOrCreateOMPInternalVariable(KmpCriticalNameTy, Name);
 }
 
 GlobalVariable *
 OpenMPIRBuilder::createOffloadMaptypes(SmallVectorImpl<uint64_t> &Mappings,
                                        std::string VarName) {
   llvm::Constant *MaptypesArrayInit =
       llvm::ConstantDataArray::get(M.getContext(), Mappings);
   auto *MaptypesArrayGlobal = new llvm::GlobalVariable(
       M, MaptypesArrayInit->getType(),
       /*isConstant=*/true, llvm::GlobalValue::PrivateLinkage, MaptypesArrayInit,
       VarName);
   MaptypesArrayGlobal->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
   return MaptypesArrayGlobal;
 }
 
+bool OpenMPIRBuilder::checkAndEmitFlushAfterAtomic(
+    const LocationDescription &Loc, llvm::AtomicOrdering AO, AtomicKind AK) {
+  assert(!(AO == AtomicOrdering::NotAtomic ||
+           AO == llvm::AtomicOrdering::Unordered) &&
+         "Unexpected Atomic Ordering.");
+
+  bool Flush = false;
+  llvm::AtomicOrdering FlushAO = AtomicOrdering::Monotonic;
+
+  switch (AK) {
+  case Read:
+    if (AO == AtomicOrdering::Acquire || AO == AtomicOrdering::AcquireRelease ||
+        AO == AtomicOrdering::SequentiallyConsistent) {
+      FlushAO = AtomicOrdering::Acquire;
+      Flush = true;
+    }
+    break;
+  case Write:
+  case Update:
+    if (AO == AtomicOrdering::Release || AO == AtomicOrdering::AcquireRelease ||
+        AO == AtomicOrdering::SequentiallyConsistent) {
+      FlushAO = AtomicOrdering::Release;
+      Flush = true;
+    }
+    break;
+  case Capture:
+    switch (AO) {
+    case AtomicOrdering::Acquire:
+      FlushAO = AtomicOrdering::Acquire;
+      Flush = true;
+      break;
+    case AtomicOrdering::Release:
+      FlushAO = AtomicOrdering::Release;
+      Flush = true;
+      break;
+    case AtomicOrdering::AcquireRelease:
+    case AtomicOrdering::SequentiallyConsistent:
+      FlushAO = AtomicOrdering::AcquireRelease;
+      Flush = true;
+      break;
+    default:
+      // do nothing - leave silently.
+      break;
+    }
+  }
+
+  if (Flush) {
+    // Currently Flush RT call still doesn't take memory_ordering, so for when
+    // that happens, this tries to do the resolution of which atomic ordering
+    // to use with but issue the flush call
+    // TODO: pass `FlushAO` after memory ordering support is added
+    (void)FlushAO;
+    emitFlush(Loc);
+  }
+
+  // for AO == AtomicOrdering::Monotonic and  all other case combinations
+  // do nothing
+  return Flush;
+}
+
+OpenMPIRBuilder::InsertPointTy
+OpenMPIRBuilder::createAtomicRead(const LocationDescription &Loc,
+                                  AtomicOpValue &X, AtomicOpValue &V,
+                                  AtomicOrdering AO) {
+  if (!updateToLocation(Loc))
+    return Loc.IP;
+
+  Type *XTy = X.Var->getType();
+  assert(XTy->isPointerTy() && "OMP Atomic expects a pointer to target memory");
+  Type *XElemTy = XTy->getPointerElementType();
+  assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
+          XElemTy->isPointerTy()) &&
+         "OMP atomic read expected a scalar type");
+
+  Value *XRead = nullptr;
+
+  if (XElemTy->isIntegerTy()) {
+    LoadInst *XLD =
+        Builder.CreateLoad(XElemTy, X.Var, X.IsVolatile, "omp.atomic.read");
+    XLD->setAtomic(AO);
+    XRead = cast<Value>(XLD);
+  } else {
+    // We need to bitcast and perform atomic op as integer
+    unsigned Addrspace = cast<PointerType>(XTy)->getAddressSpace();
+    IntegerType *IntCastTy =
+        IntegerType::get(M.getContext(), XElemTy->getScalarSizeInBits());
+    Value *XBCast = Builder.CreateBitCast(
+        X.Var, IntCastTy->getPointerTo(Addrspace), "atomic.src.int.cast");
+    LoadInst *XLoad =
+        Builder.CreateLoad(IntCastTy, XBCast, X.IsVolatile, "omp.atomic.load");
+    XLoad->setAtomic(AO);
+    if (XElemTy->isFloatingPointTy()) {
+      XRead = Builder.CreateBitCast(XLoad, XElemTy, "atomic.flt.cast");
+    } else {
+      XRead = Builder.CreateIntToPtr(XLoad, XElemTy, "atomic.ptr.cast");
+    }
+  }
+  checkAndEmitFlushAfterAtomic(Loc, AO, AtomicKind::Read);
+  Builder.CreateStore(XRead, V.Var, V.IsVolatile);
+  return Builder.saveIP();
+}
+
+OpenMPIRBuilder::InsertPointTy
+OpenMPIRBuilder::createAtomicWrite(const LocationDescription &Loc,
+                                   AtomicOpValue &X, Value *Expr,
+                                   AtomicOrdering AO) {
+  if (!updateToLocation(Loc))
+    return Loc.IP;
+
+  Type *XTy = X.Var->getType();
+  assert(XTy->isPointerTy() && "OMP Atomic expects a pointer to target memory");
+  Type *XElemTy = XTy->getPointerElementType();
+  assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
+          XElemTy->isPointerTy()) &&
+         "OMP atomic write expected a scalar type");
+
+  if (XElemTy->isIntegerTy()) {
+    StoreInst *XSt = Builder.CreateStore(Expr, X.Var, X.IsVolatile);
+    XSt->setAtomic(AO);
+  } else {
+    // We need to bitcast and perform atomic op as integers
+    unsigned Addrspace = cast<PointerType>(XTy)->getAddressSpace();
+    IntegerType *IntCastTy =
+        IntegerType::get(M.getContext(), XElemTy->getScalarSizeInBits());
+    Value *XBCast = Builder.CreateBitCast(
+        X.Var, IntCastTy->getPointerTo(Addrspace), "atomic.dst.int.cast");
+    Value *ExprCast =
+        Builder.CreateBitCast(Expr, IntCastTy, "atomic.src.int.cast");
+    Builder.GetInsertBlock()->getParent()->dump();
+    StoreInst *XSt = Builder.CreateStore(ExprCast, XBCast, X.IsVolatile);
+    Builder.GetInsertBlock()->dump();
+    XSt->setAtomic(AO);
+  }
+
+  checkAndEmitFlushAfterAtomic(Loc, AO, AtomicKind::Write);
+  return Builder.saveIP();
+}
+
+OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createAtomicUpdate(
+    const LocationDescription &Loc, Instruction *AllocIP, AtomicOpValue &X,
+    Value *Expr, AtomicOrdering AO, AtomicRMWInst::BinOp RMWOp,
+    AtomicUpdateCallbackTy &UpdateOp, bool IsXLHSInRHSPart) {
+  if (!updateToLocation(Loc))
+    return Loc.IP;
+
+  Type *XTy = X.Var->getType();
+  assert(XTy->isPointerTy() && "OMP Atomic expects a pointer to target memory");
+  Type *XElemTy = XTy->getPointerElementType();
+  assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
+          XElemTy->isPointerTy()) &&
+         "OMP atomic update expected a scalar type");
+  assert((RMWOp != AtomicRMWInst::Max) && (RMWOp != AtomicRMWInst::Min) &&
+         (RMWOp != AtomicRMWInst::UMax) && (RMWOp != AtomicRMWInst::UMin) &&
+         "OpenMP atomic does not support LT or GT operations");
+
+  emitAtomicUpdate(AllocIP, X.Var, Expr, AO, RMWOp, UpdateOp, X.IsVolatile,
+                   IsXLHSInRHSPart);
+  checkAndEmitFlushAfterAtomic(Loc, AO, AtomicKind::Update);
+  return Builder.saveIP();
+}
+
+Value *OpenMPIRBuilder::emitRMWOpAsInstruction(Value *Src1, Value *Src2,
+                                               AtomicRMWInst::BinOp RMWOp) {
+  switch (RMWOp) {
+  case AtomicRMWInst::Add:
+    return Builder.CreateAdd(Src1, Src2);
+  case AtomicRMWInst::Sub:
+    return Builder.CreateSub(Src1, Src2);
+  case AtomicRMWInst::And:
+    return Builder.CreateAnd(Src1, Src2);
+  case AtomicRMWInst::Nand:
+    return Builder.CreateNeg(Builder.CreateAnd(Src1, Src2));
+  case AtomicRMWInst::Or:
+    return Builder.CreateOr(Src1, Src2);
+  case AtomicRMWInst::Xor:
+    return Builder.CreateXor(Src1, Src2);
+  case AtomicRMWInst::Xchg:
+  case AtomicRMWInst::FAdd:
+  case AtomicRMWInst::FSub:
+  case AtomicRMWInst::BAD_BINOP:
+  case AtomicRMWInst::Max:
+  case AtomicRMWInst::Min:
+  case AtomicRMWInst::UMax:
+  case AtomicRMWInst::UMin:
+    llvm_unreachable("Unsupported atomic update operation");
+  }
+  llvm_unreachable("Unsupported atomic update operation");
+}
+
+std::pair<Value *, Value *>
+OpenMPIRBuilder::emitAtomicUpdate(Instruction *AllocIP, Value *X, Value *Expr,
+                                  AtomicOrdering AO, AtomicRMWInst::BinOp RMWOp,
+                                  AtomicUpdateCallbackTy &UpdateOp,
+                                  bool VolatileX, bool IsXLHSInRHSPart) {
+  Type *XElemTy = X->getType()->getPointerElementType();
+
+  bool DoCmpExch =
+      ((RMWOp == AtomicRMWInst::BAD_BINOP) || (RMWOp == AtomicRMWInst::FAdd)) ||
+      (RMWOp == AtomicRMWInst::FSub) ||
+      (RMWOp == AtomicRMWInst::Sub && !IsXLHSInRHSPart);
+
+  std::pair<Value *, Value *> Res;
+  if (XElemTy->isIntegerTy() && !DoCmpExch) {
+    Res.first = Builder.CreateAtomicRMW(RMWOp, X, Expr, llvm::MaybeAlign(), AO);
+    // not needed except in case of postfix captures. Generate anyway for
+    // consistency with the else part. Will be removed with any DCE pass.
+    Res.second = emitRMWOpAsInstruction(Res.first, Expr, RMWOp);
+  } else {
+    unsigned Addrspace = cast<PointerType>(X->getType())->getAddressSpace();
+    IntegerType *IntCastTy =
+        IntegerType::get(M.getContext(), XElemTy->getScalarSizeInBits());
+    Value *XBCast =
+        Builder.CreateBitCast(X, IntCastTy->getPointerTo(Addrspace));
+    LoadInst *OldVal =
+        Builder.CreateLoad(IntCastTy, XBCast, X->getName() + ".atomic.load");
+    OldVal->setAtomic(AO);
+    // CurBB
+    // |     /---\
+    // ContBB    |
+    // |     \---/
+    // ExitBB
+    BasicBlock *CurBB = Builder.GetInsertBlock();
+    Instruction *CurBBTI = CurBB->getTerminator();
+    CurBBTI = CurBBTI ? CurBBTI : Builder.CreateUnreachable();
+    BasicBlock *ExitBB =
+        CurBB->splitBasicBlock(CurBBTI, X->getName() + ".atomic.exit");
+    BasicBlock *ContBB = CurBB->splitBasicBlock(CurBB->getTerminator(),
+                                                X->getName() + ".atomic.cont");
+    ContBB->getTerminator()->eraseFromParent();
+    Builder.SetInsertPoint(ContBB);
+    llvm::PHINode *PHI = Builder.CreatePHI(OldVal->getType(), 2);
+    PHI->addIncoming(OldVal, CurBB);
+    AllocaInst *NewAtomicAddr = Builder.CreateAlloca(XElemTy);
+    NewAtomicAddr->setName(X->getName() + "x.new.val");
+    NewAtomicAddr->moveBefore(AllocIP);
+    IntegerType *NewAtomicCastTy =
+        IntegerType::get(M.getContext(), XElemTy->getScalarSizeInBits());
+    bool IsIntTy = XElemTy->isIntegerTy();
+    Value *NewAtomicIntAddr =
+        (IsIntTy)
+            ? NewAtomicAddr
+            : Builder.CreateBitCast(NewAtomicAddr,
+                                    NewAtomicCastTy->getPointerTo(Addrspace));
+    Value *OldExprVal = PHI;
+    if (!IsIntTy) {
+      if (XElemTy->isFloatingPointTy()) {
+        OldExprVal = Builder.CreateBitCast(PHI, XElemTy,
+                                           X->getName() + ".atomic.fltCast");
+      } else {
+        OldExprVal = Builder.CreateIntToPtr(PHI, XElemTy,
+                                            X->getName() + ".atomic.ptrCast");
+      }
+    }
+
+    Value *Upd = UpdateOp(OldExprVal, Builder);
+    Builder.CreateStore(Upd, NewAtomicAddr);
+    LoadInst *DesiredVal = Builder.CreateLoad(XElemTy, NewAtomicIntAddr);
+    Value *XAddr =
+        (IsIntTy)
+            ? X
+            : Builder.CreateBitCast(X, IntCastTy->getPointerTo(Addrspace));
+    AtomicOrdering Failure =
+        llvm::AtomicCmpXchgInst::getStrongestFailureOrdering(AO);
+    AtomicCmpXchgInst *Result = Builder.CreateAtomicCmpXchg(
+        XAddr, OldExprVal, DesiredVal, llvm::MaybeAlign(), AO, Failure);
+    Result->setVolatile(VolatileX);
+    Value *PreviousVal = Builder.CreateExtractValue(Result, /*Idxs=*/0);
+    Value *SuccessFailureVal = Builder.CreateExtractValue(Result, /*Idxs=*/1);
+    PHI->addIncoming(PreviousVal, Builder.GetInsertBlock());
+    Builder.CreateCondBr(SuccessFailureVal, ExitBB, ContBB);
+
+    Res.first = OldExprVal;
+    Res.second = Upd;
+
+    // set Insertion point in exit block
+    if (UnreachableInst *ExitTI =
+            dyn_cast<UnreachableInst>(ExitBB->getTerminator())) {
+      CurBBTI->eraseFromParent();
+      Builder.SetInsertPoint(ExitBB);
+    } else {
+      Builder.SetInsertPoint(ExitTI);
+    }
+  }
+
+  return Res;
+}
+
+OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createAtomicCapture(
+    const LocationDescription &Loc, Instruction *AllocIP, AtomicOpValue &X,
+    AtomicOpValue &V, Value *Expr, AtomicOrdering AO,
+    AtomicRMWInst::BinOp RMWOp, AtomicUpdateCallbackTy &UpdateOp,
+    bool UpdateExpr, bool IsPostfixUpdate, bool IsXLHSInRHSPart) {
+  if (!updateToLocation(Loc))
+    return Loc.IP;
+
+  Type *XTy = X.Var->getType();
+  assert(XTy->isPointerTy() && "OMP Atomic expects a pointer to target memory");
+  Type *XElemTy = XTy->getPointerElementType();
+  assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
+          XElemTy->isPointerTy()) &&
+         "OMP atomic capture expected a scalar type");
+  assert((RMWOp != AtomicRMWInst::Max) && (RMWOp != AtomicRMWInst::Min) &&
+         "OpenMP atomic does not support LT or GT operations");
+
+  // If UpdateExpr is 'x' updated with some `expr` not based on 'x',
+  // 'x' is simply atomically rewritten with 'expr'.
+  AtomicRMWInst::BinOp AtomicOp = (UpdateExpr ? RMWOp : AtomicRMWInst::Xchg);
+  std::pair<Value *, Value *> Result =
+      emitAtomicUpdate(AllocIP, X.Var, Expr, AO, AtomicOp, UpdateOp,
+                       X.IsVolatile, IsXLHSInRHSPart);
+
+  Value *CapturedVal = (IsPostfixUpdate ? Result.first : Result.second);
+  Builder.CreateStore(CapturedVal, V.Var, V.IsVolatile);
+
+  checkAndEmitFlushAfterAtomic(Loc, AO, AtomicKind::Capture);
+  return Builder.saveIP();
+}
+
 GlobalVariable *
 OpenMPIRBuilder::createOffloadMapnames(SmallVectorImpl<llvm::Constant *> &Names,
                                        std::string VarName) {
   llvm::Constant *MapNamesArrayInit = llvm::ConstantArray::get(
       llvm::ArrayType::get(
           llvm::Type::getInt8Ty(M.getContext())->getPointerTo(), Names.size()),
       Names);
   auto *MapNamesArrayGlobal = new llvm::GlobalVariable(

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

for (auto &FI : *ThenBB) {
     }
   }
   EXPECT_NE(SingleEndCI, nullptr);
   EXPECT_EQ(SingleEndCI->getNumArgOperands(), 2U);
   EXPECT_TRUE(isa<GlobalVariable>(SingleEndCI->getArgOperand(0)));
   EXPECT_EQ(SingleEndCI->getArgOperand(1), SingleEntryCI->getArgOperand(1));
 }
 
+TEST_F(OpenMPIRBuilderTest, OMPAtomicReadFlt) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  Type *Float32 = Type::getFloatTy(M->getContext());
+  AllocaInst *XVal = Builder.CreateAlloca(Float32);
+  XVal->setName("AtomicVar");
+  AllocaInst *VVal = Builder.CreateAlloca(Float32);
+  VVal->setName("AtomicRead");
+  AtomicOrdering AO = AtomicOrdering::Monotonic;
+  OpenMPIRBuilder::AtomicOpValue X = {XVal, false, false};
+  OpenMPIRBuilder::AtomicOpValue V = {VVal, false, false};
+
+  Builder.restoreIP(OMPBuilder.createAtomicRead(Loc, X, V, AO));
+
+  IntegerType *IntCastTy =
+      IntegerType::get(M->getContext(), Float32->getScalarSizeInBits());
+
+  BitCastInst *CastFrmFlt = cast<BitCastInst>(VVal->getNextNode());
+  EXPECT_EQ(CastFrmFlt->getSrcTy(), Float32->getPointerTo());
+  EXPECT_EQ(CastFrmFlt->getDestTy(), IntCastTy->getPointerTo());
+  EXPECT_EQ(CastFrmFlt->getOperand(0), XVal);
+
+  LoadInst *AtomicLoad = cast<LoadInst>(CastFrmFlt->getNextNode());
+  EXPECT_TRUE(AtomicLoad->isAtomic());
+  EXPECT_EQ(AtomicLoad->getPointerOperand(), CastFrmFlt);
+
+  BitCastInst *CastToFlt = cast<BitCastInst>(AtomicLoad->getNextNode());
+  EXPECT_EQ(CastToFlt->getSrcTy(), IntCastTy);
+  EXPECT_EQ(CastToFlt->getDestTy(), Float32);
+  EXPECT_EQ(CastToFlt->getOperand(0), AtomicLoad);
+
+  StoreInst *StoreofAtomic = cast<StoreInst>(CastToFlt->getNextNode());
+  EXPECT_EQ(StoreofAtomic->getValueOperand(), CastToFlt);
+  EXPECT_EQ(StoreofAtomic->getPointerOperand(), VVal);
+
+  Builder.CreateRetVoid();
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
+TEST_F(OpenMPIRBuilderTest, OMPAtomicReadInt) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  IntegerType *Int32 = Type::getInt32Ty(M->getContext());
+  AllocaInst *XVal = Builder.CreateAlloca(Int32);
+  XVal->setName("AtomicVar");
+  AllocaInst *VVal = Builder.CreateAlloca(Int32);
+  VVal->setName("AtomicRead");
+  AtomicOrdering AO = AtomicOrdering::Monotonic;
+  OpenMPIRBuilder::AtomicOpValue X = {XVal, false, false};
+  OpenMPIRBuilder::AtomicOpValue V = {VVal, false, false};
+
+  BasicBlock *EntryBB = BB;
+
+  Builder.restoreIP(OMPBuilder.createAtomicRead(Loc, X, V, AO));
+  LoadInst *AtomicLoad = nullptr;
+  StoreInst *StoreofAtomic = nullptr;
+
+  for (Instruction &Cur : *EntryBB) {
+    if (isa<LoadInst>(Cur)) {
+      AtomicLoad = cast<LoadInst>(&Cur);
+      if (AtomicLoad->getPointerOperand() == XVal)
+        continue;
+      AtomicLoad = nullptr;
+    } else if (isa<StoreInst>(Cur)) {
+      StoreofAtomic = cast<StoreInst>(&Cur);
+      if (StoreofAtomic->getPointerOperand() == VVal)
+        continue;
+      StoreofAtomic = nullptr;
+    }
+  }
+
+  EXPECT_NE(AtomicLoad, nullptr);
+  EXPECT_TRUE(AtomicLoad->isAtomic());
+
+  EXPECT_NE(StoreofAtomic, nullptr);
+  EXPECT_EQ(StoreofAtomic->getValueOperand(), AtomicLoad);
+
+  Builder.CreateRetVoid();
+  OMPBuilder.finalize();
+
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
+TEST_F(OpenMPIRBuilderTest, OMPAtomicWriteFlt) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  LLVMContext &Ctx = M->getContext();
+  Type *Float32 = Type::getFloatTy(Ctx);
+  AllocaInst *XVal = Builder.CreateAlloca(Float32);
+  XVal->setName("AtomicVar");
+  OpenMPIRBuilder::AtomicOpValue X = {XVal, false, false};
+  AtomicOrdering AO = AtomicOrdering::Monotonic;
+  Constant *ValToWrite = ConstantFP::get(Float32, 1.0);
+
+  Builder.restoreIP(OMPBuilder.createAtomicWrite(Loc, X, ValToWrite, AO));
+
+  IntegerType *IntCastTy =
+      IntegerType::get(M->getContext(), Float32->getScalarSizeInBits());
+
+  BitCastInst *CastFrmFlt = cast<BitCastInst>(XVal->getNextNode());
+  EXPECT_EQ(CastFrmFlt->getSrcTy(), Float32->getPointerTo());
+  EXPECT_EQ(CastFrmFlt->getDestTy(), IntCastTy->getPointerTo());
+  EXPECT_EQ(CastFrmFlt->getOperand(0), XVal);
+
+  Value *ExprCast = Builder.CreateBitCast(ValToWrite, IntCastTy);
+
+  StoreInst *StoreofAtomic = cast<StoreInst>(CastFrmFlt->getNextNode());
+  EXPECT_EQ(StoreofAtomic->getValueOperand(), ExprCast);
+  EXPECT_EQ(StoreofAtomic->getPointerOperand(), CastFrmFlt);
+  EXPECT_TRUE(StoreofAtomic->isAtomic());
+
+  Builder.CreateRetVoid();
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
+TEST_F(OpenMPIRBuilderTest, OMPAtomicWriteInt) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  LLVMContext &Ctx = M->getContext();
+  IntegerType *Int32 = Type::getInt32Ty(Ctx);
+  AllocaInst *XVal = Builder.CreateAlloca(Int32);
+  XVal->setName("AtomicVar");
+  OpenMPIRBuilder::AtomicOpValue X = {XVal, false, false};
+  AtomicOrdering AO = AtomicOrdering::Monotonic;
+  ConstantInt *ValToWrite = ConstantInt::get(Type::getInt32Ty(Ctx), 1U);
+
+  BasicBlock *EntryBB = BB;
+
+  Builder.restoreIP(OMPBuilder.createAtomicWrite(Loc, X, ValToWrite, AO));
+
+  StoreInst *StoreofAtomic = nullptr;
+
+  for (Instruction &Cur : *EntryBB) {
+    if (isa<StoreInst>(Cur)) {
+      StoreofAtomic = cast<StoreInst>(&Cur);
+      if (StoreofAtomic->getPointerOperand() == XVal)
+        continue;
+      StoreofAtomic = nullptr;
+    }
+  }
+
+  EXPECT_NE(StoreofAtomic, nullptr);
+  EXPECT_TRUE(StoreofAtomic->isAtomic());
+  EXPECT_EQ(StoreofAtomic->getValueOperand(), ValToWrite);
+
+  Builder.CreateRetVoid();
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
+TEST_F(OpenMPIRBuilderTest, OMPAtomicUpdate) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  IntegerType *Int32 = Type::getInt32Ty(M->getContext());
+  AllocaInst *XVal = Builder.CreateAlloca(Int32);
+  XVal->setName("AtomicVar");
+  Builder.CreateStore(ConstantInt::get(Type::getInt32Ty(Ctx), 0U), XVal);
+  OpenMPIRBuilder::AtomicOpValue X = {XVal, false, false};
+  AtomicOrdering AO = AtomicOrdering::Monotonic;
+  ConstantInt *ConstVal = ConstantInt::get(Type::getInt32Ty(Ctx), 1U);
+  Value *Expr = nullptr;
+  AtomicRMWInst::BinOp RMWOp = AtomicRMWInst::Sub;
+  bool IsXLHSInRHSPart = false;
+
+  BasicBlock *EntryBB = BB;
+  Instruction *AllocIP = EntryBB->getFirstNonPHI();
+  Value *Sub = nullptr;
+
+  auto UpdateOp = [&](Value *Atomic, IRBuilder<> &IRB) {
+    Sub = IRB.CreateSub(ConstVal, Atomic);
+    return Sub;
+  };
+  Builder.restoreIP(OMPBuilder.createAtomicUpdate(
+      Builder, AllocIP, X, Expr, AO, RMWOp, UpdateOp, IsXLHSInRHSPart));
+  BasicBlock *ContBB = EntryBB->getSingleSuccessor();
+  BranchInst *ContTI = dyn_cast<BranchInst>(ContBB->getTerminator());
+  EXPECT_NE(ContTI, nullptr);
+  BasicBlock *EndBB = ContTI->getSuccessor(0);
+  EXPECT_TRUE(ContTI->isConditional());
+  EXPECT_EQ(ContTI->getSuccessor(1), ContBB);
+  EXPECT_NE(EndBB, nullptr);
+
+  PHINode *Phi = dyn_cast<PHINode>(&ContBB->front());
+  EXPECT_NE(Phi, nullptr);
+  EXPECT_EQ(Phi->getNumIncomingValues(), 2U);
+  EXPECT_EQ(Phi->getIncomingBlock(0), EntryBB);
+  EXPECT_EQ(Phi->getIncomingBlock(1), ContBB);
+
+  EXPECT_EQ(Sub->getNumUses(), 1U);
+  StoreInst *St = dyn_cast<StoreInst>(Sub->user_back());
+  AllocaInst *UpdateTemp = dyn_cast<AllocaInst>(St->getPointerOperand());
+
+  ExtractValueInst *ExVI1 =
+      dyn_cast<ExtractValueInst>(Phi->getIncomingValueForBlock(ContBB));
+  EXPECT_NE(ExVI1, nullptr);
+  AtomicCmpXchgInst *CmpExchg =
+      dyn_cast<AtomicCmpXchgInst>(ExVI1->getAggregateOperand());
+  EXPECT_NE(CmpExchg, nullptr);
+  EXPECT_EQ(CmpExchg->getPointerOperand(), XVal);
+  EXPECT_EQ(CmpExchg->getCompareOperand(), Phi);
+  EXPECT_EQ(CmpExchg->getSuccessOrdering(), AtomicOrdering::Monotonic);
+
+  LoadInst *Ld = dyn_cast<LoadInst>(CmpExchg->getNewValOperand());
+  EXPECT_NE(Ld, nullptr);
+  EXPECT_EQ(UpdateTemp, Ld->getPointerOperand());
+
+  Builder.CreateRetVoid();
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
+TEST_F(OpenMPIRBuilderTest, OMPAtomicCapture) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  LLVMContext &Ctx = M->getContext();
+  IntegerType *Int32 = Type::getInt32Ty(Ctx);
+  AllocaInst *XVal = Builder.CreateAlloca(Int32);
+  XVal->setName("AtomicVar");
+  AllocaInst *VVal = Builder.CreateAlloca(Int32);
+  VVal->setName("AtomicCapTar");
+  StoreInst *Init =
+      Builder.CreateStore(ConstantInt::get(Type::getInt32Ty(Ctx), 0U), XVal);
+
+  OpenMPIRBuilder::AtomicOpValue X = {XVal, false, false};
+  OpenMPIRBuilder::AtomicOpValue V = {VVal, false, false};
+  AtomicOrdering AO = AtomicOrdering::Monotonic;
+  ConstantInt *Expr = ConstantInt::get(Type::getInt32Ty(Ctx), 1U);
+  AtomicRMWInst::BinOp RMWOp = AtomicRMWInst::Add;
+  bool IsXLHSInRHSPart = true;
+  bool IsPostfixUpdate = true;
+  bool UpdateExpr = true;
+
+  BasicBlock *EntryBB = BB;
+  Instruction *AllocIP = EntryBB->getFirstNonPHI();
+
+  // integer update - not used
+  auto UpdateOp = [&](Value *Atomic, IRBuilder<> &IRB) { return nullptr; };
+
+  Builder.restoreIP(OMPBuilder.createAtomicCapture(
+      Builder, AllocIP, X, V, Expr, AO, RMWOp, UpdateOp, UpdateExpr,
+      IsPostfixUpdate, IsXLHSInRHSPart));
+  EXPECT_EQ(EntryBB->getParent()->size(), 1U);
+  AtomicRMWInst *ARWM = dyn_cast<AtomicRMWInst>(Init->getNextNode());
+  EXPECT_NE(ARWM, nullptr);
+  EXPECT_EQ(ARWM->getPointerOperand(), XVal);
+  EXPECT_EQ(ARWM->getOperation(), RMWOp);
+  StoreInst *St = dyn_cast<StoreInst>(ARWM->user_back());
+  EXPECT_NE(St, nullptr);
+  EXPECT_EQ(St->getPointerOperand(), VVal);
+
+  Builder.CreateRetVoid();
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
 TEST_F(OpenMPIRBuilderTest, CreateSections) {
   using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
   using BodyGenCallbackTy = llvm::OpenMPIRBuilder::StorableBodyGenCallbackTy;
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
   F->setName("func");
   IRBuilder<> Builder(BB);