[Mlir-commits] [mlir] 9fb52cb - [MLIR][OpenMP] Added omp.atomic.read and omp.atomic.write

Shraiysh Vaishay llvmlistbot at llvm.org
Wed Oct 27 01:35:58 PDT 2021


Author: Shraiysh Vaishay
Date: 2021-10-27T14:05:44+05:30
New Revision: 9fb52cb3f1235639a5a357fb5de18e1aeeb5b5f4

URL: https://github.com/llvm/llvm-project/commit/9fb52cb3f1235639a5a357fb5de18e1aeeb5b5f4
DIFF: https://github.com/llvm/llvm-project/commit/9fb52cb3f1235639a5a357fb5de18e1aeeb5b5f4.diff

LOG: [MLIR][OpenMP] Added omp.atomic.read and omp.atomic.write

This patch supports the atomic construct (read and write) following
section 2.17.7 of OpenMP 5.0 standard. Also added tests and
verifier for the same.

Reviewed By: kiranchandramohan

Differential Revision: https://reviews.llvm.org/D111992

Added: 
    

Modified: 
    clang/lib/CodeGen/CGStmtOpenMP.cpp
    flang/lib/Semantics/check-omp-structure.cpp
    llvm/include/llvm/Frontend/OpenMP/OMP.td
    mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
    mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
    mlir/test/Dialect/OpenMP/invalid.mlir
    mlir/test/Dialect/OpenMP/ops.mlir

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index a3b509f91414a..991c9573f8973 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -5991,6 +5991,7 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, OpenMPClauseKind Kind,
   case OMPC_when:
   case OMPC_adjust_args:
   case OMPC_append_args:
+  case OMPC_memory_order:
     llvm_unreachable("Clause is not allowed in 'omp atomic'.");
   }
 }

diff  --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index d173d679d440e..919853792944a 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1479,6 +1479,7 @@ CHECK_SIMPLE_CLAUSE(Filter, OMPC_filter)
 CHECK_SIMPLE_CLAUSE(When, OMPC_when)
 CHECK_SIMPLE_CLAUSE(AdjustArgs, OMPC_adjust_args)
 CHECK_SIMPLE_CLAUSE(AppendArgs, OMPC_append_args)
+CHECK_SIMPLE_CLAUSE(MemoryOrder, OMPC_memory_order)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
 CHECK_REQ_SCALAR_INT_CLAUSE(NumTasks, OMPC_num_tasks)

diff  --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 551ed6915162e..fffd8d75f6a71 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -144,6 +144,26 @@ def OMPC_Schedule : Clause<"schedule"> {
   ];
 }
 
+def OMP_MEMORY_ORDER_SeqCst : ClauseVal<"seq_cst", 1, 1> {}
+def OMP_MEMORY_ORDER_AcqRel : ClauseVal<"acq_rel", 2, 1> {}
+def OMP_MEMORY_ORDER_Acquire : ClauseVal<"acquire", 3, 1> {}
+def OMP_MEMORY_ORDER_Release : ClauseVal<"release", 4, 1> {}
+def OMP_MEMORY_ORDER_Relaxed : ClauseVal<"relaxed", 5, 1> {}
+def OMP_MEMORY_ORDER_Default : ClauseVal<"default", 6, 0> {
+  let isDefault = 1;
+}
+def OMPC_MemoryOrder : Clause<"memory_order"> {
+  let enumClauseValue = "MemoryOrderKind";
+  let allowedClauseValues = [
+    OMP_MEMORY_ORDER_SeqCst,
+    OMP_MEMORY_ORDER_AcqRel,
+    OMP_MEMORY_ORDER_Acquire,
+    OMP_MEMORY_ORDER_Release,
+    OMP_MEMORY_ORDER_Relaxed,
+    OMP_MEMORY_ORDER_Default
+  ];
+}
+
 def OMPC_Ordered : Clause<"ordered"> {
   let clangClass = "OMPOrderedClause";
   let flangClass = "ScalarIntConstantExpr";

diff  --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 54649fd570efb..ed04dc4ca395d 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -499,6 +499,37 @@ def TaskwaitOp : OpenMP_Op<"taskwait"> {
   let assemblyFormat = "attr-dict";
 }
 
+//===----------------------------------------------------------------------===//
+// 2.17.7 atomic construct
+//===----------------------------------------------------------------------===//
+
+// In the OpenMP Specification, atomic construct has an `atomic-clause` which
+// can take the values `read`, `write`, `update` and `capture`. These four
+// kinds of atomic constructs are fundamentally independent and are handled
+// separately while lowering. Having four separate operations (one for each
+// value of the clause) here decomposes handling of this construct into a
+// two-step process.
+
+def AtomicReadOp : OpenMP_Op<"atomic.read"> {
+  let arguments = (ins OpenMP_PointerLikeType:$address,
+                       DefaultValuedAttr<I64Attr, "0">:$hint,
+                       OptionalAttr<MemoryOrderKind>:$memory_order);
+  let results = (outs AnyType);
+  let parser = [{ return parseAtomicReadOp(parser, result); }];
+  let printer = [{ return printAtomicReadOp(p, *this); }];
+  let verifier = [{ return verifyAtomicReadOp(*this); }];
+}
+
+def AtomicWriteOp : OpenMP_Op<"atomic.write"> {
+  let arguments = (ins OpenMP_PointerLikeType:$address,
+                       AnyType:$value,
+                       DefaultValuedAttr<I64Attr, "0">:$hint,
+                       OptionalAttr<MemoryOrderKind>:$memory_order);
+  let parser = [{ return parseAtomicWriteOp(parser, result); }];
+  let printer = [{ return printAtomicWriteOp(p, *this); }];
+  let verifier = [{ return verifyAtomicWriteOp(*this); }];
+}
+
 //===----------------------------------------------------------------------===//
 // 2.19.5.7 declare reduction Directive
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 64292b80e4ddb..14e899489f6fc 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -391,8 +391,9 @@ static LogicalResult verifyReductionVarList(Operation *op,
 ///
 /// hint-clause = `hint` `(` hint-value `)`
 static ParseResult parseSynchronizationHint(OpAsmParser &parser,
-                                            IntegerAttr &hintAttr) {
-  if (failed(parser.parseOptionalKeyword("hint"))) {
+                                            IntegerAttr &hintAttr,
+                                            bool parseKeyword = true) {
+  if (parseKeyword && failed(parser.parseOptionalKeyword("hint"))) {
     hintAttr = IntegerAttr::get(parser.getBuilder().getI64Type(), 0);
     return success();
   }
@@ -450,11 +451,11 @@ static void printSynchronizationHint(OpAsmPrinter &p, Operation *op,
 
   p << "hint(";
   llvm::interleaveComma(hints, p);
-  p << ")";
+  p << ") ";
 }
 
 /// Verifies a synchronization hint clause
-static LogicalResult verifySynchronizationHint(Operation *op, int32_t hint) {
+static LogicalResult verifySynchronizationHint(Operation *op, uint64_t hint) {
 
   // Helper function to get n-th bit from the right end of `value`
   auto bitn = [](int value, int n) -> bool { return value & (1 << n); };
@@ -492,6 +493,8 @@ enum ClauseType {
   orderClause,
   orderedClause,
   inclusiveClause,
+  memoryOrderClause,
+  hintClause,
   COUNT
 };
 
@@ -720,6 +723,19 @@ static ParseResult parseClauses(OpAsmParser &parser, OperationState &result,
         return failure();
       auto attr = UnitAttr::get(parser.getBuilder().getContext());
       result.addAttribute("inclusive", attr);
+    } else if (clauseKeyword == "memory_order") {
+      StringRef memoryOrder;
+      if (checkAllowed(memoryOrderClause) || parser.parseLParen() ||
+          parser.parseKeyword(&memoryOrder) || parser.parseRParen())
+        return failure();
+      result.addAttribute("memory_order",
+                          parser.getBuilder().getStringAttr(memoryOrder));
+    } else if (clauseKeyword == "hint") {
+      IntegerAttr hint;
+      if (checkAllowed(hintClause) ||
+          parseSynchronizationHint(parser, hint, false))
+        return failure();
+      result.addAttribute("hint", hint);
     } else {
       return parser.emitError(parser.getNameLoc())
              << clauseKeyword << " is not a valid clause";
@@ -1184,5 +1200,105 @@ static LogicalResult verifyOrderedRegionOp(OrderedRegionOp op) {
   return success();
 }
 
+//===----------------------------------------------------------------------===//
+// AtomicReadOp
+//===----------------------------------------------------------------------===//
+
+/// Parser for AtomicReadOp
+///
+/// operation ::= `omp.atomic.read` atomic-clause-list address `->` result-type
+/// address ::= operand `:` type
+static ParseResult parseAtomicReadOp(OpAsmParser &parser,
+                                     OperationState &result) {
+  OpAsmParser::OperandType address;
+  Type addressType;
+  SmallVector<ClauseType> clauses = {memoryOrderClause, hintClause};
+  SmallVector<int> segments;
+
+  if (parser.parseOperand(address) ||
+      parseClauses(parser, result, clauses, segments) ||
+      parser.parseColonType(addressType) ||
+      parser.resolveOperand(address, addressType, result.operands))
+    return failure();
+
+  SmallVector<Type> resultType;
+  if (parser.parseArrowTypeList(resultType))
+    return failure();
+  result.addTypes(resultType);
+  return success();
+}
+
+/// Printer for AtomicReadOp
+static void printAtomicReadOp(OpAsmPrinter &p, AtomicReadOp op) {
+  p << " " << op.address() << " ";
+  if (op.memory_order())
+    p << "memory_order(" << op.memory_order().getValue() << ") ";
+  if (op.hintAttr())
+    printSynchronizationHint(p << " ", op, op.hintAttr());
+  p << ": " << op.address().getType() << " -> " << op.getType();
+  return;
+}
+
+/// Verifier for AtomicReadOp
+static LogicalResult verifyAtomicReadOp(AtomicReadOp op) {
+  if (op.memory_order()) {
+    StringRef memOrder = op.memory_order().getValue();
+    if (memOrder.equals("acq_rel") || memOrder.equals("release"))
+      return op.emitError(
+          "memory-order must not be acq_rel or release for atomic reads");
+  }
+  return verifySynchronizationHint(op, op.hint());
+}
+
+//===----------------------------------------------------------------------===//
+// AtomicWriteOp
+//===----------------------------------------------------------------------===//
+
+/// Parser for AtomicWriteOp
+///
+/// operation ::= `omp.atomic.write` atomic-clause-list operands
+/// operands ::= address `,` value
+/// address ::= operand `:` type
+/// value ::= operand `:` type
+static ParseResult parseAtomicWriteOp(OpAsmParser &parser,
+                                      OperationState &result) {
+  OpAsmParser::OperandType address, value;
+  Type addrType, valueType;
+  SmallVector<ClauseType> clauses = {memoryOrderClause, hintClause};
+  SmallVector<int> segments;
+
+  if (parser.parseOperand(address) || parser.parseComma() ||
+      parser.parseOperand(value) ||
+      parseClauses(parser, result, clauses, segments) ||
+      parser.parseColonType(addrType) || parser.parseComma() ||
+      parser.parseType(valueType) ||
+      parser.resolveOperand(address, addrType, result.operands) ||
+      parser.resolveOperand(value, valueType, result.operands))
+    return failure();
+  return success();
+}
+
+/// Printer for AtomicWriteOp
+static void printAtomicWriteOp(OpAsmPrinter &p, AtomicWriteOp op) {
+  p << " " << op.address() << ", " << op.value() << " ";
+  if (op.memory_order())
+    p << "memory_order(" << op.memory_order() << ") ";
+  if (op.hintAttr())
+    printSynchronizationHint(p, op, op.hintAttr());
+  p << ": " << op.address().getType() << ", " << op.value().getType();
+  return;
+}
+
+/// Verifier for AtomicWriteOp
+static LogicalResult verifyAtomicWriteOp(AtomicWriteOp op) {
+  if (op.memory_order()) {
+    StringRef memoryOrder = op.memory_order().getValue();
+    if (memoryOrder.equals("acq_rel") || memoryOrder.equals("acquire"))
+      return op.emitError(
+          "memory-order must not be acq_rel or acquire for atomic writes");
+  }
+  return verifySynchronizationHint(op, op.hint());
+}
+
 #define GET_OP_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOps.cpp.inc"

diff  --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index a5b179d9b8227..e57ddfcbfec86 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -375,3 +375,99 @@ func @omp_ordered5(%arg1 : i32, %arg2 : i32, %arg3 : i32, %vec0 : i64, %vec1 : i
   }
   return
 }
+
+// -----
+
+func @omp_atomic_read1(%addr : memref<i32>) {
+  // expected-error @below {{the hints omp_sync_hint_nonspeculative and omp_sync_hint_speculative cannot be combined.}}
+  %1 = omp.atomic.read %addr hint(speculative, nonspeculative) : memref<i32> -> i32
+  return
+}
+
+// -----
+
+func @omp_atomic_read2(%addr : memref<i32>) {
+  // expected-error @below {{attribute 'memory_order' failed to satisfy constraint: MemoryOrderKind Clause}}
+  %1 = omp.atomic.read %addr memory_order(xyz) : memref<i32> -> i32
+  return
+}
+
+// -----
+
+func @omp_atomic_read3(%addr : memref<i32>) {
+  // expected-error @below {{memory-order must not be acq_rel or release for atomic reads}}
+  %1 = omp.atomic.read %addr memory_order(acq_rel) : memref<i32> -> i32
+  return
+}
+
+// -----
+
+func @omp_atomic_read4(%addr : memref<i32>) {
+  // expected-error @below {{memory-order must not be acq_rel or release for atomic reads}}
+  %1 = omp.atomic.read %addr memory_order(release) : memref<i32> -> i32
+  return
+}
+
+// -----
+
+func @omp_atomic_read5(%addr : memref<i32>) {
+  // expected-error @below {{at most one memory_order clause can appear on the omp.atomic.read operation}}
+  %1 = omp.atomic.read %addr memory_order(acquire) memory_order(relaxed) : memref<i32> -> i32
+  return
+}
+
+// -----
+
+func @omp_atomic_read6(%addr : memref<i32>) {
+  // expected-error @below {{at most one hint clause can appear on the omp.atomic.read operation}}
+  %1 = omp.atomic.read  %addr hint(speculative) hint(contended) : memref<i32> -> i32
+  return
+}
+
+// -----
+
+func @omp_atomic_write1(%addr : memref<i32>, %val : i32) {
+  // expected-error @below {{the hints omp_sync_hint_uncontended and omp_sync_hint_contended cannot be combined}}
+  omp.atomic.write  %addr, %val hint(contended, uncontended) : memref<i32>, i32
+  return
+}
+
+// -----
+
+func @omp_atomic_write2(%addr : memref<i32>, %val : i32) {
+  // expected-error @below {{memory-order must not be acq_rel or acquire for atomic writes}}
+  omp.atomic.write  %addr, %val memory_order(acq_rel) : memref<i32>, i32
+  return
+}
+
+// -----
+
+func @omp_atomic_write3(%addr : memref<i32>, %val : i32) {
+  // expected-error @below {{memory-order must not be acq_rel or acquire for atomic writes}}
+  omp.atomic.write  %addr, %val memory_order(acquire) : memref<i32>, i32
+  return
+}
+
+// -----
+
+func @omp_atomic_write4(%addr : memref<i32>, %val : i32) {
+  // expected-error @below {{at most one memory_order clause can appear on the omp.atomic.write operation}}
+  omp.atomic.write  %addr, %val memory_order(release) memory_order(seq_cst) : memref<i32>, i32
+  return
+}
+
+// -----
+
+func @omp_atomic_write5(%addr : memref<i32>, %val : i32) {
+  // expected-error @below {{at most one hint clause can appear on the omp.atomic.write operation}}
+  omp.atomic.write  %addr, %val hint(contended) hint(speculative) : memref<i32>, i32
+  return
+}
+
+// -----
+
+func @omp_atomic_write6(%addr : memref<i32>, %val : i32) {
+  // expected-error @below {{attribute 'memory_order' failed to satisfy constraint: MemoryOrderKind Clause}}
+  omp.atomic.write  %addr, %val memory_order(xyz) : memref<i32>, i32
+  return
+}

diff  --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index 5defcc5d91226..0d7c7af74579b 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -454,3 +454,37 @@ func @omp_ordered(%arg1 : i32, %arg2 : i32, %arg3 : i32,
 
   return
 }
+
+// CHECK-LABEL: omp_atomic_read
+// CHECK-SAME: (%[[ADDR:.*]]: memref<i32>)
+func @omp_atomic_read(%addr : memref<i32>) {
+  // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] : memref<i32> -> i32
+  %1 = omp.atomic.read %addr : memref<i32> -> i32
+  // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] memory_order(seq_cst) : memref<i32> -> i32
+  %2 = omp.atomic.read %addr memory_order(seq_cst) : memref<i32> -> i32
+  // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] memory_order(acquire) : memref<i32> -> i32
+  %5 = omp.atomic.read %addr memory_order(acquire) : memref<i32> -> i32
+  // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] memory_order(relaxed) : memref<i32> -> i32
+  %6 = omp.atomic.read %addr memory_order(relaxed) : memref<i32> -> i32
+  // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] hint(contended, nonspeculative) : memref<i32> -> i32
+  %7 = omp.atomic.read %addr hint(nonspeculative, contended) : memref<i32> -> i32
+  // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] memory_order(seq_cst) hint(contended, speculative) : memref<i32> -> i32
+  %8 = omp.atomic.read %addr hint(speculative, contended) memory_order(seq_cst) : memref<i32> -> i32
+  return
+}
+
+// CHECK-LABEL: omp_atomic_write
+// CHECK-SAME: (%[[ADDR:.*]]: memref<i32>, %[[VAL:.*]]: i32)
+func @omp_atomic_write(%addr : memref<i32>, %val : i32) {
+  // CHECK: omp.atomic.write %[[ADDR]], %[[VAL]] : memref<i32>, i32
+  omp.atomic.write %addr, %val : memref<i32>, i32
+  // CHECK: omp.atomic.write %[[ADDR]], %[[VAL]] memory_order(seq_cst) : memref<i32>, i32
+  omp.atomic.write %addr, %val memory_order(seq_cst) : memref<i32>, i32
+  // CHECK: omp.atomic.write %[[ADDR]], %[[VAL]] memory_order(release) : memref<i32>, i32
+  omp.atomic.write %addr, %val memory_order(release) : memref<i32>, i32
+  // CHECK: omp.atomic.write %[[ADDR]], %[[VAL]] memory_order(relaxed) : memref<i32>, i32
+  omp.atomic.write %addr, %val memory_order(relaxed) : memref<i32>, i32
+  // CHECK: omp.atomic.write %[[ADDR]], %[[VAL]] hint(uncontended, speculative) : memref<i32>, i32
+  omp.atomic.write %addr, %val hint(speculative, uncontended) : memref<i32>, i32
+  return
+}


        


More information about the Mlir-commits mailing list