[flang][OpenMP] Added lowering support for atomic read and write constructs

[NimishMishra]

Summary

This patch adds lowering support for atomic read and write constructs.

Co-authored-by: Kiran Chandramohan <[e-mail]>

[NimishMishra]

@rriddle @ftynse Could you please look at the MLIR side of this patch? Concretely, without the changes done there, atomic read and write constructs were giving builtin.unrealized_conversion_cast for the variables used within them. An example I give below:

program sample
     use omp_lib

     integer::x,v
     !$omp atomic read
         x = v
 end program sample

llvm.func @_QQmain() {
    %0 = llvm.mlir.constant(1 : i64) : i64
    %1 = llvm.alloca %0 x i32 {bindc_name = "v", in_type = i32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEv"} : (i64) -> !llvm.ptr<i32>
    %2 = builtin.unrealized_conversion_cast %1 : !llvm.ptr<i32> to !fir.ref<i32>
    %3 = llvm.mlir.constant(1 : i64) : i64
    %4 = llvm.alloca %3 x i32 {bindc_name = "x", in_type = i32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEx"} : (i64) -> !llvm.ptr<i32>
    %5 = builtin.unrealized_conversion_cast %4 : !llvm.ptr<i32> to !fir.ref<i32>
    omp.atomic.read %5 = %2   : !fir.ref<i32>
    llvm.return
  }

We technically allowed the conversion for these operations (having no region attached to them).

[shraiysh]

flang/lib/Lower/OpenMP.cpp

473

Make this ClauseMemoryOrderKindAttr instead of StringAttr, and accept ClauseMemoryOrderKindAttr in the genOmpAtomic..Clauses(). Do not create a StringAttr internally, instead directly create a ClauseMemoryOrderKindAttr using the corresponding enum.

504

Thi check is not necessary. Just replace hint ? nullptr : hint with hint.

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

49

Instead of individual structs, can we do something using templates like RegionlessOpConversion<T> and use it like RegionOpConversion is used. Avoids code duplication. Feel free to choose a better name, but I think it would be reusable if we could template this..

[rriddle]

The MLIR side LGTM

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

54

56

nit: Drop the newline here (same for the other pattern)

68

[kiranchandramohan]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

3370–3376

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

102–103

Can you add the new legalisation conditions here as well? It might be better for code reuse and the keep things in sync to create a new function in this file for the legalisation conditions and call that from CodeGen.cpp in Flang.

Add a test with the relevant op from the memref dialect if it is supported i.e an OpenMP to LLVM conversion test with an omp.atomic.read op having the memref type as its argument.

[NimishMishra]

Addressed comments

[NimishMishra]

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

54

I do not understand why but this change isn't working. A lot of errors come in, but they stem from OpAdaptor not declared. I even tried with typename omp::AtomicReadOp::OpAdaptor adaptor and that didn't work.

102–103

I don't think the memref dialect is supported as yet. If I give a memref argument to omp.atomic.read, it just says that's an incompatible argument.

[peixin]

flang/test/Lower/OpenMP/atomic02.f90

2

Please use bbc here. The same reason in https://reviews.llvm.org/D122595 (Check the most recent comment).

[NimishMishra]

Shifted tests to use bbc instead of flang-new

[shraiysh]

LGTM

flang/lib/Lower/OpenMP.cpp

380

We won't need foundMemoryOrderClause as an argument if we would take ClauseMemoryOrderKindAttr instead of ClauseMemoryOrderKind in the function arguments. ClauseMemoryOrderKindAttr can stay nullptr when the memory order clause is not found.

467

Change this to ClauseMemoryOrderKindAttr.

500–502

Remove this condition once the other changes for memory_order are done.

mlir/include/mlir/Conversion/OpenMPToLLVM/ConvertOpenMPToLLVM.h

22–24

Is this required? This looks like a function that is only used in this cpp file only.

[NimishMishra]

Addressed comments.

mlir/include/mlir/Conversion/OpenMPToLLVM/ConvertOpenMPToLLVM.h

22–24

Yes. This function is also called from CodeGen.cpp

[peixin]

flang/lib/Lower/OpenMP.cpp

389

Why nullptr for hint(0)?

flang/test/Lower/OpenMP/atomic02.f90

17

nit: c7_i32 -> [[CONST_7]]?

[NimishMishra]

flang/lib/Lower/OpenMP.cpp

389

The FIR generation is not distinguishing between hint(0) and nullptr. So when I pass a hint(0) and emit FIR, it ends up as hint() and causes problems during further lowering. That's why I have made this change to make no distinction between a hint(0) and an absence of hint clause.

[clementval]

flang/lib/Optimizer/Dialect/FIRType.cpp

15

I find it weird to have OpenMP things coming into FIR types.

[clementval]

flang/lib/Optimizer/Dialect/FIRType.cpp

15

To me is kind of go against the concept of distinct dialects. What do you think @schweitz?

[schweitz]

flang/lib/Optimizer/Dialect/FIRType.cpp

15

I agree. The FIR dialect should not depend on the OpenMP dialect any more than Fortran language depends on the OpenMP definition.

[peixin]

flang/lib/Lower/OpenMP.cpp

389

It seems that this is one bug here. Will post it on slack. It is also related to critical construct.

[schweitz]

flang/lib/Lower/OpenMP.cpp

389

Do you want to use a function other than ToInt64 to distinguish between a 0 and the absence of an argument? Or is the problem that an absent argument is always promoted to 0 before it gets here?

flang/lib/Optimizer/Dialect/FIRType.cpp

939

It probably makes sense to move this pointer modeling code into the "common code for tools" area and not add a dependency between FIR dialect and OpenMP dialect here.

[clementval]

Remove dependency between dialects.

[rriddle]

mlir/include/mlir/Conversion/OpenMPToLLVM/ConvertOpenMPToLLVM.h

11

Is this include necessary?

22

Please use proper sentences for documentation.

23

Can you nest this in a different namespace or rename it?

This is a very generically named function to live in mlir.

[shraiysh]

flang/lib/Lower/OpenMP.cpp

389

Do you want to use a function other than ToInt64 to distinguish between a 0 and the absence of an argument?

I don't think that is required but fir-dev treats these as separate cases, which is the cause of confusion. I will add it here too at the moment for consistency.

[NimishMishra]

flang/lib/Optimizer/Dialect/FIRType.cpp

939

I understand that we need to decouple things here. However, can you explain a little where exactly this "common code for tools" are is? I don't suppose it would be flang/lib/Common, would it be?

[kiranchandramohan]

flang/lib/Optimizer/Dialect/FIRType.cpp

939

It could be include/flang/Optimizer/Support/InitFIR.h or the related cpp file if by tools @schweitz meant the driver tools (bbc,tco,flang-new etc).

mlir/include/mlir/Conversion/OpenMPToLLVM/ConvertOpenMPToLLVM.h

23

The GPU dialect uses a name like configureGpuToNVVMConversionLegality(ConversionTarget &target). Can we do something similar?

[schweitz]

flang/lib/Optimizer/Dialect/FIRType.cpp

939

I was thinking of include/flang/Tools or possibly a new tools/? subdirectory.

[NimishMishra]

flang/lib/Lower/OpenMP.cpp

389

This problem is resolved through the currently under-review patch https://reviews.llvm.org/D123186

mlir/include/mlir/Conversion/OpenMPToLLVM/ConvertOpenMPToLLVM.h

11

I think so. We wanted to have ConversionTarget visible here in order to define the function we did below.

[kiranchandramohan]

flang/include/flang/Tools/PointerModels.h

1 ↗

(On Diff #421834)

Please add the licensing and other headers as well as the include guards.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

3365

Please update this comment with info about operations without regions as well. Or may be we can just say legalize OpenMP operations here and move the details to comments in the called function.

[rriddle]

mlir/include/mlir/Conversion/OpenMPToLLVM/ConvertOpenMPToLLVM.h

11

Please prefer using forward declarations instead of includes in header files. It doesn't look like the full definition of ConversionTarget is needed here.

[NimishMishra]

Addressed comments.

[kiranchandramohan]

flang/lib/Optimizer/Dialect/FIRType.cpp

15

@schweitz @clementval Is this location OK?

939

Is the change OK? Or do you want to move the attachInterface also to another place?

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

102–103

I see some code attaching memref to the PointerLikeModel. Can you check again?
https://github.com/llvm/llvm-project/blob/dd56939a4b04072a449a05701373e95d00cc494f/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp#L59

[peixin]

The CI fails. Can this be at high priority? The threadprivate directive lowering needs some code of this PR. The default clause lowering needs some code from the threadprivate directive lowering.

[NimishMishra]

Rebased.

[NimishMishra]

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

102–103

I am trying to attach the following test case in flang/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir

func @atomic_read(%v: memref<i32>, %x: memref<i32>){
       omp.atomic.read %v = %x : memref <i32>
       return
}

The error I get is that operation operand must be OpenMP-compatible variable type, but got !llvm.struct<(ptr<i32>, ptr<i32>, i64)>. If we further investigate the operands to the atomic operation, they lower to unrealised conversion cast operations.

[NimishMishra]

Fixed CI failures/

[kiranchandramohan]

LGTM. Please wait for approval from @clementval/@schweitz for the location of attaching the OpenMP interface and any outstanding comments from the other reviewers.

flang/include/flang/Tools/PointerModels.h

1 ↗

(On Diff #423812)

Nit: I think this can just be PointerModels.h or Tools/PointerModels.h.

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

102–103

Ahh OK. I missed that point. We will need to do something similar to what OpenACC does in handling these memref types so that the LLVM conversion works fine for them. But it is definitely not in the scope of this patch.
https://github.com/llvm/llvm-project/blob/d7565de6cc6ba4de8a5d73282281ff95d7a0ad46/mlir/lib/Conversion/OpenACCToLLVM/OpenACCToLLVM.cpp#L100

[schweitz]

In D122725#3461702, @kiranchandramohan wrote:

LGTM. Please wait for approval from @clementval/@schweitz for the location of attaching the OpenMP interface and any outstanding comments from the other reviewers.

I'm good with the location of PointerModels.h. Thanks for breaking the dependence.

[clementval]

In D122725#3461702, @kiranchandramohan wrote:

LGTM. Please wait for approval from @clementval/@schweitz for the location of attaching the OpenMP interface and any outstanding comments from the other reviewers.

Ok for me too.

flang/lib/Lower/OpenMP.cpp

genOMP(Fortran::lower::AbstractConverter &converter,
   } else if (dir == llvm::omp::Directive::OMPD_sections) {
     auto sectionsOp = firOpBuilder.create<mlir::omp::SectionsOp>(
         currentLocation, reductionVars, /*reductions = */ nullptr,
         allocateOperands, allocatorOperands, noWaitClauseOperand);
     createBodyOfOp<omp::SectionsOp>(sectionsOp, firOpBuilder, currentLocation);
   }
 }
 
+static void genOmpAtomicHintAndMemoryOrderClauses(
+    Fortran::lower::AbstractConverter &converter,
+    const Fortran::parser::OmpAtomicClauseList &clauseList,
+    mlir::IntegerAttr &hint, mlir::omp::ClauseMemoryOrderKind &memory_order,
+    bool &foundMemoryOrderClause) {
+  auto &firOpBuilder = converter.getFirOpBuilder();
+  for (const auto &clause : clauseList.v) {
+    if (auto ompClause = std::get_if<Fortran::parser::OmpClause>(&clause.u)) {
+      if (auto hintClause =
+              std::get_if<Fortran::parser::OmpClause::Hint>(&ompClause->u)) {
+        const auto *expr = Fortran::semantics::GetExpr(hintClause->v);
+        uint64_t hintExprValue = *Fortran::evaluate::ToInt64(*expr);
+        if (hintExprValue == 0)
+          hint = nullptr;
+        else
+          hint = firOpBuilder.getI64IntegerAttr(hintExprValue);
+      }
+    } else if (auto ompMemoryOrderClause =
+                   std::get_if<Fortran::parser::OmpMemoryOrderClause>(
+                       &clause.u)) {
+      foundMemoryOrderClause = true;
+      if (std::get_if<Fortran::parser::OmpClause::Acquire>(
+              &ompMemoryOrderClause->v.u)) {
+        memory_order = omp::ClauseMemoryOrderKind::Acquire;
+      } else if (std::get_if<Fortran::parser::OmpClause::Relaxed>(
+                     &ompMemoryOrderClause->v.u)) {
+        memory_order = omp::ClauseMemoryOrderKind::Relaxed;
+      } else if (std::get_if<Fortran::parser::OmpClause::SeqCst>(
+                     &ompMemoryOrderClause->v.u)) {
+        memory_order = omp::ClauseMemoryOrderKind::Seq_cst;
+      } else if (std::get_if<Fortran::parser::OmpClause::Release>(
+                     &ompMemoryOrderClause->v.u)) {
+        memory_order = omp::ClauseMemoryOrderKind::Release;
+      }
+    }
+  }
+}
+
+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();
+  mlir::Value address;
+  // 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::ClauseMemoryOrderKind memory_order;
+  bool foundMemoryOrderClause{false};
+  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);
+  Fortran::lower::StatementContext stmtCtx;
+  auto value = fir::getBase(converter.genExprValue(
+      *Fortran::semantics::GetExpr(assignmentStmtExpr), stmtCtx));
+  if (auto varDesignator = std::get_if<
+          Fortran::common::Indirection<Fortran::parser::Designator>>(
+          &assignmentStmtVariable.u)) {
+    if (const auto *name = getDesignatorNameIfDataRef(varDesignator->value())) {
+      address = converter.getSymbolAddress(*name->symbol);
+    }
+  }
+
+  genOmpAtomicHintAndMemoryOrderClauses(converter, leftHandClauseList, hint,
+                                        memory_order, foundMemoryOrderClause);
+  genOmpAtomicHintAndMemoryOrderClauses(converter, rightHandClauseList, hint,
+                                        memory_order, foundMemoryOrderClause);
+  firOpBuilder.create<mlir::omp::AtomicWriteOp>(
+      currentLocation, address, value, hint,
+
+      foundMemoryOrderClause ? mlir::omp::ClauseMemoryOrderKindAttr::get(
+                                   firOpBuilder.getContext(), memory_order)
+                             : nullptr);
+}
+
+static void genOmpAtomicRead(Fortran::lower::AbstractConverter &converter,
+                             Fortran::lower::pft::Evaluation &eval,
+                             const Fortran::parser::OmpAtomicRead &atomicRead) {
+  auto &firOpBuilder = converter.getFirOpBuilder();
+  auto currentLocation = converter.getCurrentLocation();
+  mlir::Value to_address;
+  mlir::Value from_address;
+  // 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::ClauseMemoryOrderKind memory_order;
+  bool foundMemoryOrderClause{false};
+  const Fortran::parser::OmpAtomicClauseList &rightHandClauseList =
+      std::get<2>(atomicRead.t);
+  const Fortran::parser::OmpAtomicClauseList &leftHandClauseList =
+      std::get<0>(atomicRead.t);
+  const auto &assignmentStmtExpr =
+      std::get<Fortran::parser::Expr>(std::get<3>(atomicRead.t).statement.t);
+  const auto &assignmentStmtVariable = std::get<Fortran::parser::Variable>(
+      std::get<3>(atomicRead.t).statement.t);
+  if (auto exprDesignator = std::get_if<
+          Fortran::common::Indirection<Fortran::parser::Designator>>(
+          &assignmentStmtExpr.u)) {
+    if (const auto *name =
+            getDesignatorNameIfDataRef(exprDesignator->value())) {
+      from_address = converter.getSymbolAddress(*name->symbol);
+    }
+  }
+
+  if (auto varDesignator = std::get_if<
+          Fortran::common::Indirection<Fortran::parser::Designator>>(
+          &assignmentStmtVariable.u)) {
+    if (const auto *name = getDesignatorNameIfDataRef(varDesignator->value())) {
+      to_address = converter.getSymbolAddress(*name->symbol);
+    }
+  }
+
+  genOmpAtomicHintAndMemoryOrderClauses(converter, leftHandClauseList, hint,
+                                        memory_order, foundMemoryOrderClause);
+  genOmpAtomicHintAndMemoryOrderClauses(converter, rightHandClauseList, hint,
+                                        memory_order, foundMemoryOrderClause);
+  firOpBuilder.create<mlir::omp::AtomicReadOp>(
+      currentLocation, from_address, to_address, hint,
+      foundMemoryOrderClause ? mlir::omp::ClauseMemoryOrderKindAttr::get(
+                                   firOpBuilder.getContext(), memory_order)
+                             : 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 auto &) {
+                   TODO(converter.getCurrentLocation(),
+                        "Atomic update & 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
std::visit(
           [&](const Fortran::parser::OpenMPExecutableAllocate
                   &execAllocConstruct) {
             TODO(converter.getCurrentLocation(), "OpenMPExecutableAllocate");
           },
           [&](const Fortran::parser::OpenMPBlockConstruct &blockConstruct) {
             genOMP(converter, eval, blockConstruct);
           },
           [&](const Fortran::parser::OpenMPAtomicConstruct &atomicConstruct) {
-            TODO(converter.getCurrentLocation(), "OpenMPAtomicConstruct");
+            genOMP(converter, eval, atomicConstruct);
           },
           [&](const Fortran::parser::OpenMPCriticalConstruct
                   &criticalConstruct) {
             genOMP(converter, eval, criticalConstruct);
           },
       },
       ompConstruct.u);
 }

flang/lib/Optimizer/CodeGen/CodeGen.cpp

void runOnOperation() override final {
     mlir::populateFuncToLLVMConversionPatterns(typeConverter, pattern);
     mlir::populateOpenMPToLLVMConversionPatterns(typeConverter, pattern);
     mlir::arith::populateArithmeticToLLVMConversionPatterns(typeConverter,
                                                             pattern);
     mlir::cf::populateControlFlowToLLVMConversionPatterns(typeConverter,
                                                           pattern);
     mlir::ConversionTarget target{*context};
     target.addLegalDialect<mlir::LLVM::LLVMDialect>();
     // The OpenMP dialect is legal for Operations without regions, for those
     // which contains regions it is legal if the region contains only the
     // LLVM dialect.
-    target.addDynamicallyLegalOp<mlir::omp::ParallelOp, mlir::omp::WsLoopOp,
-                                 mlir::omp::MasterOp>([&](Operation *op) {
-      return typeConverter.isLegal(&op->getRegion(0));
-    });
+
+    mlir::addDynamicallyLegalOperations(target, typeConverter);
     target.addLegalDialect<mlir::omp::OpenMPDialect>();
 
     // required NOPs for applying a full conversion
     target.addLegalOp<mlir::ModuleOp>();
 
     // apply the patterns
     if (mlir::failed(mlir::applyFullConversion(getModule(), target,
                                                std::move(pattern)))) {
       signalPassFailure();
     }
   }
 
 private:
   fir::FIRToLLVMPassOptions options;
 };

flang/lib/Optimizer/Dialect/FIRType.cpp

 //===-- FIRType.cpp -------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Optimizer/Dialect/FIRType.h"
 #include "flang/Optimizer/Dialect/FIRDialect.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinDialect.h"
 #include "mlir/IR/Diagnostics.h"
 #include "mlir/IR/DialectImplementation.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Support/ErrorHandling.h"
return tuple && tuple.size() == 2 &&
           (acceptRawFunc && tuple.getType(0).isa<mlir::FunctionType>())) &&
          fir::isa_integer(tuple.getType(1));
 }
 
 //===----------------------------------------------------------------------===//
 // FIROpsDialect
 //===----------------------------------------------------------------------===//
 
+namespace {
+/// Model for FIR pointer like types that already provide a `getElementType`
+/// method
+template <typename T>
+struct PointerLikeModel
+    : public mlir::omp::PointerLikeType::ExternalModel<PointerLikeModel<T>, T> {
+  mlir::Type getElementType(mlir::Type pointer) const {
+    return pointer.cast<T>().getElementType();
+  }
+};
+
+template <typename T>
+struct AlternativePointerLikeModel
+    : public mlir::omp::PointerLikeType::ExternalModel<
+          AlternativePointerLikeModel<T>, T> {
+  mlir::Type getElementType(mlir::Type pointer) const {
+    return pointer.cast<T>().getEleTy();
+  }
+};
+
+} // end namespace
 void FIROpsDialect::registerTypes() {
   addTypes<BoxType, BoxCharType, BoxProcType, CharacterType, fir::ComplexType,
            FieldType, HeapType, fir::IntegerType, LenType, LogicalType,
            LLVMPointerType, PointerType, RealType, RecordType, ReferenceType,
            SequenceType, ShapeType, ShapeShiftType, ShiftType, SliceType,
            TypeDescType, fir::VectorType>();
+  fir::ReferenceType::attachInterface<PointerLikeModel<fir::ReferenceType>>(
+      *getContext());
+
+  fir::PointerType::attachInterface<PointerLikeModel<fir::PointerType>>(
+      *getContext());
+
+  fir::HeapType::attachInterface<AlternativePointerLikeModel<fir::HeapType>>(
+      *getContext());
+
+  fir::LLVMPointerType::attachInterface<
+      AlternativePointerLikeModel<fir::LLVMPointerType>>(*getContext());
 }

flang/test/Lower/OpenMP/atomic01.f90

+! RUN: bbc -fopenmp -emit-fir %s -o - | \
+! RUN: FileCheck %s --check-prefix=FIRDialect
+! RUN: bbc -fopenmp %s -o - | tco --disable-llvm --print-ir-after=fir-to-llvm-ir 2>& 1 | \
+! RUN: FileCheck %s --check-prefix=LLVMDialect
+
+! This test checks the lowering of atomic read
+
+!FIRDialect: func @_QQmain() {
+!FIRDialect: %[[VAR_A:.*]] = fir.address_of(@_QFEa) : !fir.ref<!fir.char<1>>
+!FIRDialect: %[[VAR_B:.*]] = fir.address_of(@_QFEb) : !fir.ref<!fir.char<1>>
+!FIRDialect: %[[VAR_C:.*]] = fir.alloca !fir.logical<4> {bindc_name = "c", uniq_name = "_QFEc"}
+!FIRDialect: %[[VAR_D:.*]] = fir.alloca !fir.logical<4> {bindc_name = "d", uniq_name = "_QFEd"}
+!FIRDialect: %[[VAR_E:.*]] = fir.address_of(@_QFEe) : !fir.ref<!fir.char<1,8>>
+!FIRDialect: %[[VAR_F:.*]] = fir.address_of(@_QFEf) : !fir.ref<!fir.char<1,8>>
+!FIRDialect: %[[VAR_G:.*]] = fir.alloca f32 {bindc_name = "g", uniq_name = "_QFEg"}
+!FIRDialect: %[[VAR_H:.*]] = fir.alloca f32 {bindc_name = "h", uniq_name = "_QFEh"}
+!FIRDialect: %[[VAR_X:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"}
+!FIRDialect: %[[VAR_Y:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFEy"}
+!FIRDialect: omp.atomic.read %[[VAR_X]] = %[[VAR_Y]] memory_order(acquire)  hint(uncontended) : !fir.ref<i32>
+!FIRDialect: omp.atomic.read %[[VAR_A]] = %[[VAR_B]] memory_order(relaxed)  : !fir.ref<!fir.char<1>>
+!FIRDialect: omp.atomic.read %[[VAR_C]] = %[[VAR_D]] memory_order(seq_cst)  hint(contended) : !fir.ref<!fir.logical<4>>
+!FIRDialect: omp.atomic.read %[[VAR_E]] = %[[VAR_F]] hint(speculative) : !fir.ref<!fir.char<1,8>>
+!FIRDialect: omp.atomic.read %[[VAR_G]] = %[[VAR_H]] hint(nonspeculative) : !fir.ref<f32>
+!FIRDialect: omp.atomic.read %[[VAR_G]] = %[[VAR_H]] : !fir.ref<f32>
+!FIRDialect: return
+!FIRDialect: }
+
+!LLVMDialect: llvm.func @_QQmain() {
+!LLVMDialect: %[[LLVM_VAR_A:.*]] = llvm.mlir.addressof @_QFEa : !llvm.ptr<array<1 x i8>>
+!LLVMDialect: %[[LLVM_VAR_B:.*]] = llvm.mlir.addressof @_QFEb : !llvm.ptr<array<1 x i8>>
+!LLVMDialect: {{.*}} = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect:  %[[LLVM_VAR_C:.*]] = llvm.alloca {{.*}} x i32 {bindc_name = "c", in_type = !fir.logical<4>, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEc"} : (i64) -> !llvm.ptr<i32>
+!LLVMDialect: {{.*}} = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect: %[[LLVM_VAR_D:.*]] = llvm.alloca {{.*}} x i32 {bindc_name = "d", in_type = !fir.logical<4>, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEd"} : (i64) -> !llvm.ptr<i32>
+!LLVMDialect: %[[LLVM_VAR_E:.*]] = llvm.mlir.addressof @_QFEe : !llvm.ptr<array<8 x i8>>
+!LLVMDialect: %[[LLVM_VAR_F:.*]] = llvm.mlir.addressof @_QFEf : !llvm.ptr<array<8 x i8>>
+!LLVMDialect: {{.*}} = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect: %[[LLVM_VAR_G:.*]] = llvm.alloca {{.*}} x f32 {bindc_name = "g", in_type = f32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEg"} : (i64) -> !llvm.ptr<f32>
+!LLVMDialect: {{.*}} = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect: %[[LLVM_VAR_H:.*]] = llvm.alloca {{.*}} x f32 {bindc_name = "h", in_type = f32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEh"} : (i64) -> !llvm.ptr<f32>
+!LLVMDialect: {{.*}} = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect: %[[LLVM_VAR_X:.*]] = llvm.alloca {{.*}} x i32 {bindc_name = "x", in_type = i32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEx"} : (i64) -> !llvm.ptr<i32>
+!LLVMDialect: {{.*}} = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect: %[[LLVM_VAR_Y:.*]] = llvm.alloca {{.*}} x i32 {bindc_name = "y", in_type = i32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEy"} : (i64) -> !llvm.ptr<i32>
+!LLVMDialect: omp.atomic.read %[[LLVM_VAR_X]] = %[[LLVM_VAR_Y]] memory_order(acquire)  hint(uncontended) : !llvm.ptr<i32>
+!LLVMDialect: omp.atomic.read %[[LLVM_VAR_A]] =  %[[LLVM_VAR_B]] memory_order(relaxed) : !llvm.ptr<array<1 x i8>>
+!LLVMDialect: omp.atomic.read %[[LLVM_VAR_C]] = %[[LLVM_VAR_D]] memory_order(seq_cst)  hint(contended) : !llvm.ptr<i32>
+!LLVMDialect: omp.atomic.read %[[LLVM_VAR_E]] = %[[LLVM_VAR_F]] hint(speculative) : !llvm.ptr<array<8 x i8>>
+!LLVMDialect: omp.atomic.read %[[LLVM_VAR_G]] = %[[LLVM_VAR_H]] hint(nonspeculative) : !llvm.ptr<f32>
+!LLVMDialect: omp.atomic.read %[[LLVM_VAR_G]] = %[[LLVM_VAR_H]] : !llvm.ptr<f32>
+!LLVMDialect: llvm.return
+!LLVMDialect: }
+
+program OmpAtomic
+
+    use omp_lib
+    integer :: x, y
+    character :: a, b
+    logical :: c, d
+    character(8) :: e, f
+    real g, h
+    !$omp atomic acquire read hint(omp_sync_hint_uncontended)
+       x = y
+    !$omp atomic relaxed read hint(omp_sync_hint_none)
+       a = b
+    !$omp atomic read seq_cst hint(omp_sync_hint_contended)
+       c = d
+    !$omp atomic read hint(omp_sync_hint_speculative)
+       e = f
+    !$omp atomic read hint(omp_sync_hint_nonspeculative)
+       g = h
+    !$omp atomic read
+       g = h
+end program OmpAtomic

flang/test/Lower/OpenMP/atomic02.f90

+! RUN: bbc -fopenmp -emit-fir %s -o - | \
+! RUN: FileCheck %s --check-prefix=FIRDialect
+! RUN: bbc -fopenmp %s -o - | tco --disable-llvm --print-ir-after=fir-to-llvm-ir 2>& 1 | \
+! RUN: FileCheck %s --check-prefix=LLVMDialect
+
+! This test checks the lowering of atomic write
+
+!FIRDialect: func @_QQmain() {
+!FIRDialect: %[[VAR_X:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"}
+!FIRDialect: %[[VAR_Y:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFEy"}
+!FIRDialect: %[[VAR_Z:.*]] = fir.alloca i32 {bindc_name = "z", uniq_name = "_QFEz"}
+!FIRDialect: %[[CONST_44:.*]] = arith.constant 44 : i32
+!FIRDialect: omp.atomic.write %[[VAR_X]] = %[[CONST_44]] hint(uncontended) memory_order(seq_cst) : !fir.ref<i32>, i32
+!FIRDialect: %[[CONST_7:.*]] = arith.constant 7 : i32
+!FIRDialect: {{.*}} = fir.load %[[VAR_Y]] : !fir.ref<i32>
+!FIRDialect: %[[VAR_7y:.*]] = arith.muli %c7_i32, {{.*}} : i32
+!FIRDialect: omp.atomic.write %[[VAR_X]] = %[[VAR_7y]] memory_order(relaxed) : !fir.ref<i32>, i32
+!FIRDialect: %[[CONST_10:.*]] = arith.constant 10 : i32
+!FIRDialect: {{.*}} = fir.load %[[VAR_X]] : !fir.ref<i32>
+!FIRDialect: {{.*}} = arith.muli %[[CONST_10]], {{.*}} : i32
+!FIRDialect: {{.*}} = fir.load %[[VAR_Z]] : !fir.ref<i32>
+!FIRDialect: %[[CONST_2:.*]] = arith.constant 2 : i32
+!FIRDialect: {{.*}} = arith.divsi {{.*}}, %[[CONST_2]] : i32
+!FIRDialect: {{.*}} = arith.addi {{.*}}, {{.*}} : i32
+!FIRDialect: omp.atomic.write %[[VAR_Y]] = {{.*}} hint(speculative) memory_order(release) : !fir.ref<i32>, i32
+!FIRDialect: return
+!FIRDialect: }
+
+!LLVMDialect: llvm.func @_QQmain() {
+!LLVMDialect: %[[LLVM_VAR_44:.*]] = llvm.mlir.constant(44 : i32) : i32
+!LLVMDialect: %[[LLVM_VAR_7:.*]] = llvm.mlir.constant(7 : i32) : i32
+!LLVMDialect: %[[LLVM_VAR_10:.*]] = llvm.mlir.constant(10 : i32) : i32
+!LLVMDialect: %[[LLVM_VAR_2:.*]] = llvm.mlir.constant(2 : i32) : i32
+!LLVMDialect: %[[LLVM_VAR_1:.*]] = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect: %[[LLVM_VAR_X:.*]] = llvm.alloca {{.*}} x i32 {bindc_name = "x", in_type = i32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEx"} : (i64) -> !llvm.ptr<i32>
+!LLVMDialect: %[[LLVM_VAR_1_SECOND:.*]] = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect: %[[LLVM_VAR_Y:.*]] = llvm.alloca {{.*}} x i32 {bindc_name = "y", in_type = i32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEy"} : (i64) -> !llvm.ptr<i32>
+!LLVMDialect: %[[LLVM_VAR_1_THIRD:.*]] = llvm.mlir.constant(1 : i64) : i64
+!LLVMDialect: %[[LLVM_VAR_Z:.*]] = llvm.alloca {{.*}} x i32 {bindc_name = "z", in_type = i32, operand_segment_sizes = dense<0> : vector<2xi32>, uniq_name = "_QFEz"} : (i64) -> !llvm.ptr<i32>
+!LLVMDialect: omp.atomic.write %[[LLVM_VAR_X]] = %[[LLVM_VAR_44]] hint(uncontended) memory_order(seq_cst) : !llvm.ptr<i32>, i32
+!LLVMDialect: {{.*}} = llvm.load %[[LLVM_VAR_Y]] : !llvm.ptr<i32>
+!LLVMDialect: %[[LLVM_VAR_MUL_RESULT:.*]] = llvm.mul {{.*}}, %[[LLVM_VAR_7]] : i32
+!LLVMDialect: omp.atomic.write %[[LLVM_VAR_X]] = %[[LLVM_VAR_MUL_RESULT]] memory_order(relaxed) : !llvm.ptr<i32>, i32
+!LLVMDialect: {{.*}} = llvm.load %[[LLVM_VAR_X]] : !llvm.ptr<i32>
+!LLVMDialect: {{.*}} = llvm.mul {{.*}}, %[[LLVM_VAR_10]] : i32
+!LLVMDialect: {{.*}} = llvm.load %[[LLVM_VAR_Z]] : !llvm.ptr<i32>
+!LLVMDialect: {{.*}} = llvm.sdiv {{.*}}, %[[LLVM_VAR_2]] : i32
+!LLVMDialect: {{.*}} = llvm.add {{.*}} : i32
+!LLVMDialect: omp.atomic.write %[[LLVM_VAR_Y]] = {{.*}} hint(speculative) memory_order(release) : !llvm.ptr<i32>, i32
+!LLVMDialect: llvm.return
+!LLVMDialect: }
+
+program OmpAtomicWrite
+    use omp_lib
+    integer :: x, y, z
+    !$omp atomic seq_cst write hint(omp_sync_hint_uncontended)
+        x = 8*4 + 12
+
+    !$omp atomic write relaxed
+        x = 7 * y
+
+    !$omp atomic write release hint(omp_sync_hint_speculative)
+        y = 10*x + z/2
+end program OmpAtomicWrite

mlir/include/mlir/Conversion/OpenMPToLLVM/ConvertOpenMPToLLVM.h

 //===- OpenMPToLLVM.h - Utils to convert from the OpenMP dialect ----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #ifndef MLIR_CONVERSION_OPENMPTOLLVM_CONVERTOPENMPTOLLVM_H
 #define MLIR_CONVERSION_OPENMPTOLLVM_CONVERTOPENMPTOLLVM_H
 
+#include "mlir/Conversion/LLVMCommon/ConversionTarget.h"
 #include <memory>
 
 namespace mlir {
 class LLVMTypeConverter;
 class MLIRContext;
 class ModuleOp;
 template <typename T>
 class OperationPass;
 class RewritePatternSet;
 
+/// Legalise operations for a given target
+void addDynamicallyLegalOperations(ConversionTarget &target,
+                                   LLVMTypeConverter &typeConverter);
+
 /// Populate the given list with patterns that convert from OpenMP to LLVM.
 void populateOpenMPToLLVMConversionPatterns(LLVMTypeConverter &converter,
                                             RewritePatternSet &patterns);
 
 /// Create a pass to convert OpenMP operations to the LLVMIR dialect.
 std::unique_ptr<OperationPass<ModuleOp>> createConvertOpenMPToLLVMPass();
 
 } // namespace mlir
 
 #endif // MLIR_CONVERSION_OPENMPTOLLVM_CONVERTOPENMPTOLLVM_H

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

matchAndRewrite(OpType curOp, typename OpType::Adaptor adaptor,
     if (failed(rewriter.convertRegionTypes(&newOp.region(),
                                            *this->getTypeConverter())))
       return failure();
 
     rewriter.eraseOp(curOp);
     return success();
   }
 };
+
+template <typename T>
+struct RegionLessOpConversion : public ConvertOpToLLVMPattern<T> {
+  using ConvertOpToLLVMPattern<T>::ConvertOpToLLVMPattern;
+  LogicalResult
+  matchAndRewrite(T curOp, typename T::Adaptor adaptor,
+                  ConversionPatternRewriter &rewriter) const override {
+    rewriter.replaceOpWithNewOp<T>(curOp, TypeRange(), adaptor.getOperands(),
+                                   curOp->getAttrs());
+    return success();
+  }
+};
 } // namespace
 
+void mlir::addDynamicallyLegalOperations(ConversionTarget &target,
+                                         LLVMTypeConverter &typeConverter) {
+  target.addDynamicallyLegalOp<mlir::omp::ParallelOp, mlir::omp::WsLoopOp,
+                               mlir::omp::MasterOp>(
+      [&](Operation *op) { return typeConverter.isLegal(&op->getRegion(0)); });
+  target
+      .addDynamicallyLegalOp<mlir::omp::AtomicReadOp, mlir::omp::AtomicWriteOp>(
+          [&](Operation *op) {
+            return typeConverter.isLegal(op->getOperandTypes());
+          });
+}
+
 void mlir::populateOpenMPToLLVMConversionPatterns(LLVMTypeConverter &converter,
                                                   RewritePatternSet &patterns) {
   patterns.add<RegionOpConversion<omp::MasterOp>,
                RegionOpConversion<omp::ParallelOp>,
-               RegionOpConversion<omp::WsLoopOp>>(converter);
+               RegionOpConversion<omp::WsLoopOp>,
+               RegionLessOpConversion<omp::AtomicReadOp>,
+               RegionLessOpConversion<omp::AtomicWriteOp>>(converter);
 }
 
 namespace {
 struct ConvertOpenMPToLLVMPass
     : public ConvertOpenMPToLLVMBase<ConvertOpenMPToLLVMPass> {
   void runOnOperation() override;
 };
 } // namespace
 
 void ConvertOpenMPToLLVMPass::runOnOperation() {
   auto module = getOperation();
 
   // Convert to OpenMP operations with LLVM IR dialect
   RewritePatternSet patterns(&getContext());
   LLVMTypeConverter converter(&getContext());
   arith::populateArithmeticToLLVMConversionPatterns(converter, patterns);
   cf::populateControlFlowToLLVMConversionPatterns(converter, patterns);
   populateMemRefToLLVMConversionPatterns(converter, patterns);
   populateFuncToLLVMConversionPatterns(converter, patterns);
   populateOpenMPToLLVMConversionPatterns(converter, patterns);
 
   LLVMConversionTarget target(getContext());
-  target.addDynamicallyLegalOp<omp::MasterOp, omp::ParallelOp, omp::WsLoopOp>(
-      [&](Operation *op) { return converter.isLegal(&op->getRegion(0)); });
   target.addLegalOp<omp::TerminatorOp, omp::TaskyieldOp, omp::FlushOp,
                     omp::BarrierOp, omp::TaskwaitOp>();
+  addDynamicallyLegalOperations(target, converter);
   if (failed(applyPartialConversion(module, target, std::move(patterns))))
     signalPassFailure();
 }
 
 std::unique_ptr<OperationPass<ModuleOp>> mlir::createConvertOpenMPToLLVMPass() {
   return std::make_unique<ConvertOpenMPToLLVMPass>();
 }