[flang][OpenMP] Lowering support for atomic update construct

[NimishMishra]

Summary

This patch adds lowering support for atomic update construct. A region is associated with every omp.atomic.update operation wherein resides: (1) the evaluation of the expression on the RHS of the atomic assignment statement, and (2) a omp.yield operation that yields the extended value of expression evaluated in (1).

[peixin]

Please check D125793. This patch should fail for pointers. If this fails, please implement atomic update depending on D125793.

[shraiysh]

nit: name the test file to atomic-update.f90 in accordance with the recent changes by Peixin.

Minor changes requested. Thanks!

flang/lib/Lower/OpenMP.cpp

1188–1189

nit: move declarations closer to their use.

flang/test/Lower/OpenMP/atomic03.f90

48 ↗

(On Diff #429682)

Can we please add more testcases. There are a many possibilities with memory order and hint and it would be nice to test most of them (ideally all of them). Having more testcases that test different scenarios can't hurt us. [Suggestion] Maybe we can separate them into different subroutines (based on memory order) for clarity. Feel free to separate them however you'd like to ensure readability.

[peixin]

flang/test/Lower/OpenMP/atomic03.f90

48 ↗

(On Diff #429682)

Please add one more test case for pointer similar to D125793.

[peixin]

flang/lib/Lower/OpenMP.cpp

1202

This should be incorrect for pointers. You may use the following code:

mlir::Value address = fir::getBase(converter.genExprAddr(
      *Fortran::semantics::GetExpr(assignmentStmtVariable), stmtCtx));

[NimishMishra]

Addressed comments

[kiranchandramohan]

Please add a description to the summary of the patch to describe what is being implemented in the patch along with a brief description of how it is implemented.

flang/lib/Lower/OpenMP.cpp

283

Nit: please document this option.

300

Would it be better to pass the mlir::Value result as an argument instead of Expr and also reuse it below at the terminator insertion?

349

Nit: expand auto

1220

Nit: Can this function share a lot of code with the genOmpAtomicUpdate? The hint clause setting, getting the designator, creating the update op etc seem to be similar?

1253

Can you add a comment here on why the omp::AtomicUpdateOp is created here? Something like the following.

"If atomic-clause is not present on the construct, the behavior is as if the update clause is specified."

flang/test/Lower/OpenMP/atomic-update.f90

2

Nit: please test with the flang driver as well.

2

Nit: atomic and atomic update constructs.

4

Nit: can this be on the same line as the one above?

47–50

If the options in {{.*}} are significant then capture in a variable and use it in the operation where it is used. If it is not significant then please remove.

This comment applies to all the tests.

[NimishMishra]

flang/lib/Lower/OpenMP.cpp

300

This is causing a change.

If earlier the IR was

omp.atomic.update %[[VAR_Y]] : !fir.ref<i32> {
   ^bb0(%[[ARG:.*]]: i32):
    fir.store %[[ARG]] to %[[TEMP_9]] : !fir.ref<i32>
    %4 = fir.load %[[TEMP_9]] : !fir.ref<i32>
    %5 = arith.constant 1 : i32
    %[[RESULT:.*]] = arith.addi %4, %5 : i32
     omp.yield(%[[RESULT]] : i32)
}

Now it becomes:

%4 = fir.load %[[TEMP_9]] : !fir.ref<i32>
%5 = arith.constant 1 : i32
%[[RESULT:.*]] = arith.addi %4, %5 : i32
omp.atomic.update %[[VAR_Y]] : !fir.ref<i32> {
   ^bb0(%[[ARG:.*]]: i32):
    fir.store %[[ARG]] to %[[TEMP_9]] : !fir.ref<i32>
     omp.yield(%[[RESULT]] : i32)
}

This does seem OK to me. But still if you could confirm it once.

[shraiysh]

flang/lib/Lower/OpenMP.cpp

300

This is not accurate IR.

The "before" IR was correct (not optimal but correct). The value would get updated with that IR.

The "after" IR is both incorrect and not optimal. It is loading an un-related value before the atomic update begins and so the result will not accurately "update" the value.

The standard mentions that the evaluation of expr in x += expr is atomic. In this case, expr is %5, not %4 or %[[RESULT]]. Hence the evaluation of %5 can be outside the construct, but not of %[[RESULT]].

Solution: The best way to do this would be to avoid generation of load-store operations and directly use the argument. This load-store is required for worksharing loops but it would be great if we could avoid generating them for update. The arith.addi operation must be inside the update region. If it seems very hard to avoid load-store then the "before" IR is at least correct.

[shraiysh]

flang/lib/Lower/OpenMP.cpp

300

The standard mentions that the evaluation of expr in x += expr is atomic. In this case, expr is %5, not %4 or %[[RESULT]]. Hence the evaluation of %5 can be outside the construct, but not of %[[RESULT]].

Correction (apologies for mistyping some stuff) - The standard mentions that only the evaluation of expr in x += expr need not be atomic, but the update to x must be atomic. In this case, expr is %5, not %4 or %[[RESULT]]. Hence the evaluation of %5 can be outside the construct, but both %4 and %[[RESULT]] must be inside the atomic region.

This still does not change the solution and the fact that the "after" IR is incorrect.

[NimishMishra]

Rebased and addressed comments

[peixin]

flang/lib/Lower/OpenMP.cpp

274

I am thinking if this should be split into one new single function instead of handling the special case in current createBodyOfOp, which is complex enough. The atomic update only uses part of code in createBodyOfOp, and it has nothing to do with the outerCombined and privitization.

Actually, atomic update is one kind of special case, which defines the specific region statements in parsing. It is not the general body in the atomic region. So, would it be better to create one single function to support this?

[NimishMishra]

• created a new function for atomic update lowering

• modified the generated IR to use %arg0 in the region instead of separately loading the region argument

[NimishMishra]

flang/test/Lower/OpenMP/atomic-update.f90

96

Missed this. Will fix in next diff upload

114

Missed this too. Will fix in next diff upload

[peixin]

flang/lib/Lower/OpenMP.cpp

281

// bind the argument to the symbol

This is not necessary. The code can comment itself.

289

Why do you generate the result twice?

1095

address = fir::getBase(converter.genExprAddr(
    *Fortran::semantics::GetExpr(assignmentStmtVariable), stmtCtx));

Move this out of if condition.

1111

createBodyOfAtomicUpdateOp is only used here, and it's not one large function, so it is OK to inline it directly.

[NimishMishra]

Addressed comments.

[shraiysh]

The generated code in testcases looks good. Please wait for @peixin's approval for the code part, but functionality-wise this LGTM.

[peixin]

Thanks for the update. The structure of lowering atomic update operation is much more clear now. I have a few more comments.

flang/lib/Lower/OpenMP.cpp

263

Remove the inline.

284

1091

The vector of symbols seems to be not necessary. Use one symbol is enough since update operation always has one argument.

Use one more description name -> updateSymbol, or better one.

1111

No need to pass currentLocation. It can be obtained from converter.

1112

symbolVector is from assignmentStmtVariable. You can move the code of getting the updateSymbol inside the callee.

[peixin]

flang/lib/Lower/OpenMP.cpp

263

I didn't mean add the inline attribute. What I mean is that remove this function and move the code into genOmpAtomicUpdateStatement.

[NimishMishra]

Addressed comments

[peixin]

LGTM except for a few nits.

flang/lib/Lower/OpenMP.cpp

1111

Nit: Add one empty line after 1149.

1112

Ni: Remove the empty line.

1114

Nit

1115

Ni: Remove the empty line.

1121

tiv means type of induction variable. It's not appropriate for update variable.

1122

Then you can remove the following:

tiv.push_back(varType);
locs.push_back(currentLocation);

[kazu]

I am getting:

/.../llvm-project/flang/lib/Lower/OpenMP.cpp:1134:21: error: variable 'updateSymbol' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]

If I understand the code correctly, updateSymbol is sometimes initialized. Then, we always dereference the pointer later in the function:

converter.bindSymbol(*updateSymbol, val);

I could silence the warning with:

const Fortran::semantics::Symbol *updateSymbol = nullptr;

but that wouldn't fix the fundamental problem. Would you mind taking a look? Thanks!

[NimishMishra]

In D125668#3655687, @kazu wrote:

I am getting:

/.../llvm-project/flang/lib/Lower/OpenMP.cpp:1134:21: error: variable 'updateSymbol' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]

If I understand the code correctly, updateSymbol is sometimes initialized. Then, we always dereference the pointer later in the function:

converter.bindSymbol(*updateSymbol, val);

I could silence the warning with:

const Fortran::semantics::Symbol *updateSymbol = nullptr;

but that wouldn't fix the fundamental problem. Would you mind taking a look? Thanks!

Thanks @kazu for reaching out. I have put a potential fix at https://reviews.llvm.org/D129914.

@peixin @kiranchandramohan Could you take a quick look at the fix? The issue is failing some builds with [-Werror,-Wsometimes-uninitialized]

flang/lib/Lower/OpenMP.cpp

if (eval.block) {
                "expected terminator op");
       }
     }
     if (!eval.isDirective() && eval.hasNestedEvaluations())
       createEmptyRegionBlocks(firOpBuilder, eval.getNestedEvaluations());
   }
 }
 
 /// Create the body (block) for an OpenMP Operation.
 ///
 /// \param [in]    op - the operation the body belongs to.
 /// \param [inout] converter - converter to use for the clauses.
 /// \param [in]    loc - location in source code.
 /// \param [in]    eval - current PFT node/evaluation.
 /// \oaran [in]    clauses - list of clauses to process.
 /// \param [in]    args - block arguments (induction variable[s]) for the
 ////                      region.
 /// \param [in]    outerCombined - is this an outer operation - prevents
 ///                                privatization.
+/// \param [in]     expr - the expression whose evaluation's extended
+////                        value is required
 template <typename Op>
 static void
 createBodyOfOp(Op &op, Fortran::lower::AbstractConverter &converter,
                mlir::Location &loc, Fortran::lower::pft::Evaluation &eval,
                const Fortran::parser::OmpClauseList *clauses = nullptr,
                const SmallVector<const Fortran::semantics::Symbol *> &args = {},
-               bool outerCombined = false) {
+               bool outerCombined = false,
+               const Fortran::parser::Expr *expr = nullptr) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   // If an argument for the region is provided then create the block with that
   // argument. Also update the symbol's address with the mlir argument value.
   // e.g. For loops the argument is the induction variable. And all further
   // uses of the induction variable should use this mlir value.
   mlir::Operation *storeOp = nullptr;
   if (args.size()) {
     std::size_t loopVarTypeSize = 0;
     for (const Fortran::semantics::Symbol *arg : args)
       loopVarTypeSize = std::max(loopVarTypeSize, arg->GetUltimate().size());
-    mlir::Type loopVarType = getLoopVarType(converter, loopVarTypeSize);
+    mlir::Type varType;
+    if constexpr (std::is_same_v<Op, omp::AtomicUpdateOp>) {
+      Fortran::lower::StatementContext statementCtx;
+      mlir::Value result = fir::getBase(converter.genExprValue(
+          *Fortran::semantics::GetExpr(*expr), statementCtx));
+      varType = result.getType();
+    } else {
+      varType = getLoopVarType(converter, loopVarTypeSize);
+    }
     SmallVector<Type> tiv;
     SmallVector<Location> locs;
     for (int i = 0; i < (int)args.size(); i++) {
-      tiv.push_back(loopVarType);
+      tiv.push_back(varType);
       locs.push_back(loc);
     }
     firOpBuilder.createBlock(&op.getRegion(), {}, tiv, locs);
+    if constexpr (!std::is_same_v<Op, omp::AtomicUpdateOp>) {
+      // No need to create a temporary for the argument in case of
+      // omp::AtomicUpdateOp
       int argIndex = 0;
       // The argument is not currently in memory, so make a temporary for the
       // argument, and store it there, then bind that location to the argument.
       for (const Fortran::semantics::Symbol *arg : args) {
         mlir::Value val =
             fir::getBase(op.getRegion().front().getArgument(argIndex));
         mlir::Value temp = firOpBuilder.createTemporary(
-          loc, loopVarType,
+            loc, varType,
             llvm::ArrayRef<mlir::NamedAttribute>{
                 Fortran::lower::getAdaptToByRefAttr(firOpBuilder)});
         storeOp = firOpBuilder.create<fir::StoreOp>(loc, val, temp);
         converter.bindSymbol(*arg, temp);
         argIndex++;
       }
+    }
   } else {
     firOpBuilder.createBlock(&op.getRegion());
   }
   // Set the insert for the terminator operation to go at the end of the
   // block - this is either empty or the block with the stores above,
   // the end of the block works for both.
   mlir::Block &block = op.getRegion().back();
   firOpBuilder.setInsertionPointToEnd(&block);
 
   // If it is an unstructured region and is not the outer region of a combined
   // construct, create empty blocks for all evaluations.
   if (eval.lowerAsUnstructured() && !outerCombined)
     createEmptyRegionBlocks(firOpBuilder, eval.getNestedEvaluations());
 
   // Insert the terminator.
   if constexpr (std::is_same_v<Op, omp::WsLoopOp> ||
                 std::is_same_v<Op, omp::SimdLoopOp>) {
     mlir::ValueRange results;
     firOpBuilder.create<mlir::omp::YieldOp>(loc, results);
+  } else if constexpr (std::is_same_v<Op, omp::AtomicUpdateOp>) {
+    Fortran::lower::StatementContext statementCtx;
+    mlir::Value result = fir::getBase(converter.genExprValue(
+        *Fortran::semantics::GetExpr(*expr), statementCtx));
+    firOpBuilder.create<mlir::omp::YieldOp>(loc, result);
   } else {
     firOpBuilder.create<mlir::omp::TerminatorOp>(loc);
   }
 
   // Reset the insert point to before the terminator.
   if (storeOp)
     firOpBuilder.setInsertionPointAfter(storeOp);
   else
if (auto ompClause = std::get_if<Fortran::parser::OmpClause>(&clause.u)) {
                      &ompMemoryOrderClause->v.u)) {
         memory_order = mlir::omp::ClauseMemoryOrderKindAttr::get(
             firOpBuilder.getContext(), omp::ClauseMemoryOrderKind::Release);
       }
     }
   }
 }
 
+static void genOmpAtomicUpdateStatement(
+    Fortran::lower::AbstractConverter &converter,
+    Fortran::lower::pft::Evaluation &eval,
+    const Fortran::parser::Variable &assignmentStmtVariable,
+    const Fortran::parser::Expr &assignmentStmtExpr,
+    const Fortran::parser::OmpAtomicClauseList *leftHandClauseList,
+    const Fortran::parser::OmpAtomicClauseList *rightHandClauseList) {
+  // Generate `omp.atomic.update` operation for atomic assignment statements
+  auto &firOpBuilder = converter.getFirOpBuilder();
+  auto currentLocation = converter.getCurrentLocation();
+  mlir::Value address;
+  SmallVector<const Fortran::semantics::Symbol *> symbolVector;
+  Fortran::lower::StatementContext stmtCtx;
+  if (auto varDesignator = std::get_if<
+          Fortran::common::Indirection<Fortran::parser::Designator>>(
+          &assignmentStmtVariable.u)) {
+    if (const auto *name = getDesignatorNameIfDataRef(varDesignator->value())) {
+      address = fir::getBase(converter.genExprAddr(
+          *Fortran::semantics::GetExpr(assignmentStmtVariable), stmtCtx));
+      symbolVector.push_back(name->symbol);
+    }
+  }
+  // If no hint clause is specified, the effect is as if
+  // hint(omp_sync_hint_none) had been specified.
+  mlir::IntegerAttr hint = nullptr;
+  mlir::omp::ClauseMemoryOrderKindAttr memory_order = nullptr;
+  if (leftHandClauseList)
+    genOmpAtomicHintAndMemoryOrderClauses(converter, *leftHandClauseList, hint,
+                                          memory_order);
+  if (rightHandClauseList)
+    genOmpAtomicHintAndMemoryOrderClauses(converter, *rightHandClauseList, hint,
+                                          memory_order);
+  auto atomicUpdateOp = firOpBuilder.create<mlir::omp::AtomicUpdateOp>(
+      currentLocation, address, hint, memory_order);
+  createBodyOfOp<omp::AtomicUpdateOp>(atomicUpdateOp, converter,
+                                      currentLocation, eval, nullptr,
+                                      symbolVector, false, &assignmentStmtExpr);
+}
+
 static void
 genOmpAtomicWrite(Fortran::lower::AbstractConverter &converter,
                   Fortran::lower::pft::Evaluation &eval,
                   const Fortran::parser::OmpAtomicWrite &atomicWrite) {
   auto &firOpBuilder = converter.getFirOpBuilder();
   auto currentLocation = converter.getCurrentLocation();
   // Get the value and address of atomic write operands.
   const Fortran::parser::OmpAtomicClauseList &rightHandClauseList =
       std::get<2>(atomicWrite.t);
   const Fortran::parser::OmpAtomicClauseList &leftHandClauseList =
       std::get<0>(atomicWrite.t);
   const auto &assignmentStmtExpr =
       std::get<Fortran::parser::Expr>(std::get<3>(atomicWrite.t).statement.t);
   const auto &assignmentStmtVariable = std::get<Fortran::parser::Variable>(
       std::get<3>(atomicWrite.t).statement.t);
genOmpAtomicHintAndMemoryOrderClauses(converter, leftHandClauseList, hint,
                                         memory_order);
   genOmpAtomicHintAndMemoryOrderClauses(converter, rightHandClauseList, hint,
                                         memory_order);
   firOpBuilder.create<mlir::omp::AtomicReadOp>(currentLocation, from_address,
                                                to_address, hint, memory_order);
 }
 
 static void
+genOmpAtomicUpdate(Fortran::lower::AbstractConverter &converter,
+                   Fortran::lower::pft::Evaluation &eval,
+                   const Fortran::parser::OmpAtomicUpdate &atomicUpdate) {
+  const Fortran::parser::OmpAtomicClauseList &rightHandClauseList =
+      std::get<2>(atomicUpdate.t);
+  const Fortran::parser::OmpAtomicClauseList &leftHandClauseList =
+      std::get<0>(atomicUpdate.t);
+  const auto &assignmentStmtExpr =
+      std::get<Fortran::parser::Expr>(std::get<3>(atomicUpdate.t).statement.t);
+  const auto &assignmentStmtVariable = std::get<Fortran::parser::Variable>(
+      std::get<3>(atomicUpdate.t).statement.t);
+
+  genOmpAtomicUpdateStatement(converter, eval, assignmentStmtVariable,
+                              assignmentStmtExpr, &leftHandClauseList,
+                              &rightHandClauseList);
+}
+
+static void genOmpAtomic(Fortran::lower::AbstractConverter &converter,
+                         Fortran::lower::pft::Evaluation &eval,
+                         const Fortran::parser::OmpAtomic &atomicConstruct) {
+  const Fortran::parser::OmpAtomicClauseList &atomicClauseList =
+      std::get<Fortran::parser::OmpAtomicClauseList>(atomicConstruct.t);
+  const auto &assignmentStmtExpr = std::get<Fortran::parser::Expr>(
+      std::get<Fortran::parser::Statement<Fortran::parser::AssignmentStmt>>(
+          atomicConstruct.t)
+          .statement.t);
+  const auto &assignmentStmtVariable = std::get<Fortran::parser::Variable>(
+      std::get<Fortran::parser::Statement<Fortran::parser::AssignmentStmt>>(
+          atomicConstruct.t)
+          .statement.t);
+  // If atomic-clause is not present on the construct, the behaviour is as if
+  // the update clause is specified
+  genOmpAtomicUpdateStatement(converter, eval, assignmentStmtVariable,
+                              assignmentStmtExpr, &atomicClauseList, nullptr);
+}
+
+static void
 genOMP(Fortran::lower::AbstractConverter &converter,
        Fortran::lower::pft::Evaluation &eval,
        const Fortran::parser::OpenMPAtomicConstruct &atomicConstruct) {
   std::visit(Fortran::common::visitors{
                  [&](const Fortran::parser::OmpAtomicRead &atomicRead) {
                    genOmpAtomicRead(converter, eval, atomicRead);
                  },
                  [&](const Fortran::parser::OmpAtomicWrite &atomicWrite) {
                    genOmpAtomicWrite(converter, eval, atomicWrite);
                  },
+                 [&](const Fortran::parser::OmpAtomic &atomicConstruct) {
+                   genOmpAtomic(converter, eval, atomicConstruct);
+                 },
+                 [&](const Fortran::parser::OmpAtomicUpdate &atomicUpdate) {
+                   genOmpAtomicUpdate(converter, eval, atomicUpdate);
+                 },
                  [&](const auto &) {
-                   TODO(converter.getCurrentLocation(),
-                        "Atomic update & capture");
+                   TODO(converter.getCurrentLocation(), "Atomic capture");
                  },
              },
              atomicConstruct.u);
 }
 
 void Fortran::lower::genOpenMPConstruct(
     Fortran::lower::AbstractConverter &converter,
     Fortran::lower::pft::Evaluation &eval,
     const Fortran::parser::OpenMPConstruct &ompConstruct) {
 
   std::visit(
       common::visitors{
           [&](const Fortran::parser::OpenMPStandaloneConstruct
                   &standaloneConstruct) {
             genOMP(converter, eval, standaloneConstruct);
           },
           [&](const Fortran::parser::OpenMPSectionsConstruct
                   &sectionsConstruct) {
             genOMP(converter, eval, sectionsConstruct);
           },
           [&](const Fortran::parser::OpenMPSectionConstruct &sectionConstruct) {
             genOMP(converter, eval, sectionConstruct);
           },
           [&](const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
             genOMP(converter, eval, loopConstruct);
           },
           [&](const Fortran::parser::OpenMPDeclarativeAllocate

flang/test/Lower/OpenMP/atomic-update.f90

+! This test checks lowering of atomic and atomic update constructs
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+program OmpAtomicUpdate
+    use omp_lib
+    integer :: x, y, z
+    integer, pointer :: a, b
+    integer, target :: c, d
+    a=>c
+    b=>d
+
+!CHECK: func.func @_QQmain() {
+!CHECK: %[[A:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "a", uniq_name = "_QFEa"}
+!CHECK: %[[A_ADDR:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFEa.addr"}
+!CHECK: %{{.*}} = fir.zero_bits !fir.ptr<i32>
+!CHECK: fir.store %{{.*}} to %[[A_ADDR]] : !fir.ref<!fir.ptr<i32>>
+!CHECK: %[[B:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "b", uniq_name = "_QFEb"}
+!CHECK: %[[B_ADDR:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFEb.addr"}
+!CHECK: %{{.*}} = fir.zero_bits !fir.ptr<i32>
+!CHECK: fir.store %{{.*}} to %[[B_ADDR]] : !fir.ref<!fir.ptr<i32>>
+!CHECK: %[[C_ADDR:.*]] = fir.address_of(@_QFEc) : !fir.ref<i32>
+!CHECK: %[[D_ADDR:.*]] = fir.address_of(@_QFEd) : !fir.ref<i32>
+!CHECK: %[[X:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"}
+!CHECK: %[[Y:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFEy"}
+!CHECK: %[[Z:.*]] = fir.alloca i32 {bindc_name = "z", uniq_name = "_QFEz"}
+!CHECK: %{{.*}} = fir.convert %[[C_ADDR]] : (!fir.ref<i32>) -> !fir.ptr<i32>
+!CHECK: fir.store %{{.*}} to %[[A_ADDR]] : !fir.ref<!fir.ptr<i32>>
+!CHECK: %{{.*}} = fir.convert %[[D_ADDR]] : (!fir.ref<i32>) -> !fir.ptr<i32>
+!CHECK: fir.store {{.*}} to %[[B_ADDR]] : !fir.ref<!fir.ptr<i32>>
+!CHECK: %[[LOADED_A:.*]] = fir.load %[[A_ADDR]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:  omp.atomic.update   %[[LOADED_A]] : !fir.ptr<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %[[LOADED_A:.*]] = fir.load %[[A_ADDR]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:    %{{.*}} = fir.load %[[LOADED_A]] : !fir.ptr<i32>
+!CHECK:    %[[LOADED_B:.*]] = fir.load %[[B_ADDR]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:    %{{.*}} = fir.load %[[LOADED_B]] : !fir.ptr<i32>
+!CHECK:    %[[RESULT:.*]] = arith.addi %{{.*}}, %{{.*}} : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK: }
+    !$omp atomic update
+        a = a + b 
+
+!CHECK: omp.atomic.update   %[[Y]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %[[LOADED_Y:.*]] = fir.load %[[Y]] : !fir.ref<i32>
+!CHECK:    {{.*}} = arith.constant 1 : i32
+!CHECK:    %[[RESULT:.*]] = arith.addi %[[LOADED_Y]], {{.*}} : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+!CHECK:  omp.atomic.update   %[[Z]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %[[LOADED_X:.*]] = fir.load %[[X]] : !fir.ref<i32>
+!CHECK:    %[[LOADED_Z:.*]] = fir.load %[[Z]] : !fir.ref<i32>
+!CHECK:    %[[RESULT:.*]] = arith.muli %[[LOADED_X]], %[[LOADED_Z]] : i32
+!CHECK:    omp.yield(%16 : i32)
+!CHECK:  }
+    !$omp atomic 
+        y = y + 1
+    !$omp atomic update
+        z = x * z 
+
+!CHECK:  omp.atomic.update   memory_order(relaxed) hint(uncontended) %[[X]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %[[LOADED_X:.*]] = fir.load %[[X]] : !fir.ref<i32>
+!CHECK:    %{{.*}} = arith.constant 1 : i32
+!CHECK:    %[[RESULT:.*]] = arith.subi %[[LOADED_X]], {{.*}} : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+!CHECK:  omp.atomic.update   memory_order(relaxed) %[[Y]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %[[LOADED_X:.*]] = fir.load %[[X]] : !fir.ref<i32>
+!CHECK:    %[[LOADED_Y:.*]] = fir.load %[[Y]] : !fir.ref<i32>
+!CHECK:    %[[LOADED_Z:.*]] = fir.load %[[Z]] : !fir.ref<i32>
+!CHECK:    %{{.*}} = arith.cmpi sgt, %[[LOADED_X]], %[[LOADED_Y]] : i32
+!CHECK:    %{{.*}} = arith.select %{{.*}}, %[[LOADED_X]], %[[LOADED_Y]] : i32
+!CHECK:    %{{.*}} = arith.cmpi sgt, %{{.*}}, %[[LOADED_Z]] : i32
+!CHECK:    %[[RESULT:.*]] = arith.select %{{.*}}, %{{.*}}, %[[LOADED_Z]] : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+!CHECK:  omp.atomic.update   memory_order(relaxed) hint(contended) %[[Z]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %[[LOADED_Z:.*]] = fir.load %[[Z]] : !fir.ref<i32>
+!CHECK:    %[[LOADED_X:.*]] = fir.load %[[X]] : !fir.ref<i32>
+!CHECK:    %[[RESULT:.*]] = arith.addi %[[LOADED_Z]], %[[LOADED_X]] : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+    !$omp atomic relaxed update hint(omp_sync_hint_uncontended)
+        x = x - 1
+    !$omp atomic update relaxed 
+        y = max(x, y, z)
+    !$omp atomic relaxed hint(omp_sync_hint_contended)
+        z = z + x
+
+!CHECK:  omp.atomic.update   memory_order(release) hint(contended) %[[Z]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %{{.*}} = arith.constant 10 : i32
+!CHECK:    %[[LOADED_Z:.*]] = fir.load %[[Z]] : !fir.ref<i32>
+!CHECK:   %[[RESULT:.*]] = arith.muli {{.*}}, %[[LOADED_Z]] : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+!CHECK:  omp.atomic.update   memory_order(release) hint(speculative) %[[X]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%arg0: i32):
+!CHECK:    %[[LOADED_X:.*]] = fir.load %[[X]] : !fir.ref<i32>
+!CHECK:    %[[LOADED_Z:.*]] = fir.load %[[Z]] : !fir.ref<i32>
+!CHECK:    %[[RESULT:.*]] = arith.divsi %[[LOADED_X]], %[[LOADED_Z]] : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+
+    !$omp atomic release update hint(omp_lock_hint_contended)
+        z = z * 10
+    !$omp atomic hint(omp_lock_hint_speculative) update release
+        x = x / z
+
+!CHECK:  omp.atomic.update   memory_order(seq_cst) hint(nonspeculative) %[[Y]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %{{.*}} = arith.constant 10 : i32
+!CHECK:    %[[LOADED_Y:.*]] = fir.load %[[Y]] : !fir.ref<i32>
+!CHECK:    %[[RESULT:.*]] = arith.addi %{{.*}}, %[[LOADED_Y]] : i32
+!CHECK:   omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+!CHECK:  omp.atomic.update   memory_order(seq_cst) %[[Z]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%arg0: i32):
+!CHECK:    %[[LOADED_Y:.*]] = fir.load %[[Y]] : !fir.ref<i32>
+!CHECK:    %[[LOADED_Z:.*]] = fir.load %[[Z]] : !fir.ref<i32>
+!CHECK:    %[[RESULT:.*]] = arith.addi %[[LOADED_Y]], %[[LOADED_Z]] : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+!CHECK:  return
+!CHECK: }
+    !$omp atomic hint(omp_sync_hint_nonspeculative) seq_cst
+        y = 10 + y
+    !$omp atomic seq_cst update
+        z = y + z
+end program OmpAtomicUpdate