[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

1250–1251

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

1264

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

321

Nit: please document this option.

337

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?

374

Nit: expand auto

1282

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?

1315

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

337

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

337

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

337

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

314

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

321

// bind the argument to the symbol

This is not necessary. The code can comment itself.

329

Why do you generate the result twice?

1134

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

Move this out of if condition.

1150

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

303

Remove the inline.

324

1130

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.

1150

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

1151

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

[peixin]

flang/lib/Lower/OpenMP.cpp

303

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

1150

Nit: Add one empty line after 1149.

1151

Ni: Remove the empty line.

1153

Nit

1154

Ni: Remove the empty line.

1160

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

1161

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.
 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) {
   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);
     SmallVector<Type> tiv;
     SmallVector<Location> locs;
     for (int i = 0; i < (int)args.size(); i++) {
       tiv.push_back(loopVarType);
       locs.push_back(loc);
     }
     firOpBuilder.createBlock(&op.getRegion(), {}, tiv, locs);
     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(
createBodyOfOp(Op &op, Fortran::lower::AbstractConverter &converter,
 
   // 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 {
     firOpBuilder.create<mlir::omp::TerminatorOp>(loc);
   }
 
   // Reset the insert point to before the terminator.
   if (storeOp)
     firOpBuilder.setInsertionPointAfter(storeOp);
   else
     firOpBuilder.setInsertionPointToStart(&block);
 
   // Handle privatization. Do not privatize if this is the outer operation.
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();
+  Fortran::lower::StatementContext stmtCtx;
+
+  mlir::Value address = fir::getBase(converter.genExprAddr(
+      *Fortran::semantics::GetExpr(assignmentStmtVariable), stmtCtx));
+  const Fortran::semantics::Symbol *updateSymbol;
+  if (auto varDesignator = std::get_if<
+          Fortran::common::Indirection<Fortran::parser::Designator>>(
+          &assignmentStmtVariable.u)) {
+    if (const auto *name = getDesignatorNameIfDataRef(varDesignator->value())) {
+      updateSymbol = 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);
+
+  //// Generate body of Atomic Update operation
+  // If an argument for the region is provided then create the block with that
+  // argument. Also update the symbol's address with the argument mlir value.
+  mlir::Type varType =
+      fir::getBase(
+          converter.genExprValue(
+              *Fortran::semantics::GetExpr(assignmentStmtVariable), stmtCtx))
+          .getType();
+  SmallVector<Type> varTys = {varType};
+  SmallVector<Location> locs = {currentLocation};
+  firOpBuilder.createBlock(&atomicUpdateOp.getRegion(), {}, varTys, locs);
+  mlir::Value val =
+      fir::getBase(atomicUpdateOp.getRegion().front().getArgument(0));
+  converter.bindSymbol(*updateSymbol, val);
+  // Set the insert for the terminator operation to go at the end of the
+  // block.
+  mlir::Block &block = atomicUpdateOp.getRegion().back();
+  firOpBuilder.setInsertionPointToEnd(&block);
+
+  mlir::Value result = fir::getBase(converter.genExprValue(
+      *Fortran::semantics::GetExpr(assignmentStmtExpr), stmtCtx));
+  // Insert the terminator: YieldOp.
+  firOpBuilder.create<mlir::omp::YieldOp>(currentLocation, result);
+  // Reset the insert point to before the terminator.
+  firOpBuilder.setInsertionPointToStart(&block);
+}
+
 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 =
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_B:.*]] = fir.load %[[B_ADDR]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:    %{{.*}} = fir.load %[[LOADED_B]] : !fir.ptr<i32>
+!CHECK:    %[[RESULT:.*]] = arith.addi %[[ARG]], %{{.*}} : 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:    {{.*}} = arith.constant 1 : i32
+!CHECK:    %[[RESULT:.*]] = arith.addi %[[ARG]], {{.*}} : 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:    %[[RESULT:.*]] = arith.muli %[[LOADED_X]], %[[ARG]] : i32
+!CHECK:    omp.yield(%[[RESULT]] : 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:    %{{.*}} = arith.constant 1 : i32
+!CHECK:    %[[RESULT:.*]] = arith.subi %[[ARG]], {{.*}} : 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_Z:.*]] = fir.load %[[Z]] : !fir.ref<i32>
+!CHECK:    %{{.*}} = arith.cmpi sgt, %[[LOADED_X]], %[[ARG]] : i32
+!CHECK:    %{{.*}} = arith.select %{{.*}}, %[[LOADED_X]], %[[ARG]] : 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_X:.*]] = fir.load %[[X]] : !fir.ref<i32>
+!CHECK:    %[[RESULT:.*]] = arith.addi %[[ARG]], %[[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:   %[[RESULT:.*]] = arith.muli {{.*}}, %[[ARG]] : i32
+!CHECK:    omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+!CHECK:  omp.atomic.update   memory_order(release) hint(speculative) %[[X]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %[[LOADED_Z:.*]] = fir.load %[[Z]] : !fir.ref<i32>
+!CHECK:    %[[RESULT:.*]] = arith.divsi %[[ARG]], %[[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:    %[[RESULT:.*]] = arith.addi %{{.*}}, %[[ARG]] : i32
+!CHECK:   omp.yield(%[[RESULT]] : i32)
+!CHECK:  }
+!CHECK:  omp.atomic.update   memory_order(seq_cst) %[[Z]] : !fir.ref<i32> {
+!CHECK:  ^bb0(%[[ARG:.*]]: i32):
+!CHECK:    %[[LOADED_Y:.*]] = fir.load %[[Y]] : !fir.ref<i32>
+!CHECK:    %[[RESULT:.*]] = arith.addi %[[LOADED_Y]], %[[ARG]] : 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