diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td @@ -582,8 +582,8 @@ let description = [{ This operation performs an atomic read. - The operand `address` is the address from where the value is atomically - read. + The operand `x` is the address from where the value is atomically read. + The operand `v` is the address where the value is stored after reading. `hint` is the value of hint (as specified in the hint clause). It is a compile time constant. As the name suggests, this is just a hint for @@ -593,10 +593,10 @@ can be one of `seq_cst`, `acq_rel`, `release`, `acquire` or `relaxed`. }]; - let arguments = (ins OpenMP_PointerLikeType:$address, + let arguments = (ins OpenMP_PointerLikeType:$x, + OpenMP_PointerLikeType:$v, DefaultValuedAttr:$hint, OptionalAttr:$memory_order); - let results = (outs AnyType); let parser = [{ return parseAtomicReadOp(parser, result); }]; let printer = [{ return printAtomicReadOp(p, *this); }]; let verifier = [{ return verifyAtomicReadOp(*this); }]; diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp --- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp +++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp @@ -1333,32 +1333,30 @@ /// address ::= operand `:` type static ParseResult parseAtomicReadOp(OpAsmParser &parser, OperationState &result) { - OpAsmParser::OperandType address; + OpAsmParser::OperandType x, v; Type addressType; SmallVector clauses = {memoryOrderClause, hintClause}; SmallVector segments; - if (parser.parseOperand(address) || + if (parser.parseOperand(v) || parser.parseEqual() || parser.parseOperand(x) || parseClauses(parser, result, clauses, segments) || parser.parseColonType(addressType) || - parser.resolveOperand(address, addressType, result.operands)) + parser.resolveOperand(x, addressType, result.operands) || + parser.resolveOperand(v, addressType, result.operands)) return failure(); - SmallVector 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() << " "; + p << " " << op.v() << " = " << op.x() << " "; 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(); + p << ": " << op.x().getType(); + return; } /// Verifier for AtomicReadOp @@ -1369,6 +1367,9 @@ return op.emitError( "memory-order must not be acq_rel or release for atomic reads"); } + if (op.x() == op.v()) + return op.emitError( + "read and write must not be to the same location for atomic reads"); return verifySynchronizationHint(op, op.hint()); } diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -889,23 +889,12 @@ moduleTranslation.translateLoc(opInst.getLoc(), subprogram); llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder.saveIP(), llvm::DebugLoc(diLoc)); - llvm::AtomicOrdering ao = convertAtomicOrdering(readOp.memory_order()); - llvm::Value *address = moduleTranslation.lookupValue(readOp.address()); - llvm::OpenMPIRBuilder::InsertPointTy currentIP = builder.saveIP(); - - // Insert alloca for result. - llvm::OpenMPIRBuilder::InsertPointTy allocaIP = - findAllocaInsertPoint(builder, moduleTranslation); - builder.restoreIP(allocaIP); - llvm::Value *v = builder.CreateAlloca( - moduleTranslation.convertType(readOp.getResult().getType())); - moduleTranslation.mapValue(readOp.getResult(), v); - - // Restore the IP and insert Atomic Read. - builder.restoreIP(currentIP); - llvm::OpenMPIRBuilder::AtomicOpValue atomicV = {v, false, false}; - llvm::OpenMPIRBuilder::AtomicOpValue x = {address, false, false}; - builder.restoreIP(ompBuilder->createAtomicRead(ompLoc, x, atomicV, ao)); + llvm::AtomicOrdering AO = convertAtomicOrdering(readOp.memory_order()); + llvm::Value *x = moduleTranslation.lookupValue(readOp.x()); + llvm::Value *v = moduleTranslation.lookupValue(readOp.v()); + llvm::OpenMPIRBuilder::AtomicOpValue V = {v, false, false}; + llvm::OpenMPIRBuilder::AtomicOpValue X = {x, false, false}; + builder.restoreIP(ompBuilder->createAtomicRead(ompLoc, X, V, AO)); return success(); } diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir --- a/mlir/test/Dialect/OpenMP/invalid.mlir +++ b/mlir/test/Dialect/OpenMP/invalid.mlir @@ -505,49 +505,57 @@ // ----- -func @omp_atomic_read1(%addr : memref) { +func @omp_atomic_read1(%x: memref, %v: memref) { // 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 + omp.atomic.read %v = %x hint(speculative, nonspeculative) : memref return } // ----- -func @omp_atomic_read2(%addr : memref) { +func @omp_atomic_read2(%x: memref, %v: memref) { // expected-error @below {{attribute 'memory_order' failed to satisfy constraint: MemoryOrderKind Clause}} - %1 = omp.atomic.read %addr memory_order(xyz) : memref -> i32 + omp.atomic.read %v = %x memory_order(xyz) : memref return } // ----- -func @omp_atomic_read3(%addr : memref) { +func @omp_atomic_read3(%x: memref, %v: memref) { // 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 + omp.atomic.read %v = %x memory_order(acq_rel) : memref return } // ----- -func @omp_atomic_read4(%addr : memref) { +func @omp_atomic_read4(%x: memref, %v: memref) { // 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 + omp.atomic.read %v = %x memory_order(release) : memref return } // ----- -func @omp_atomic_read5(%addr : memref) { +func @omp_atomic_read5(%x: memref, %v: memref) { // 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 + omp.atomic.read %v = %x memory_order(acquire) memory_order(relaxed) : memref return } // ----- -func @omp_atomic_read6(%addr : memref) { +func @omp_atomic_read6(%x: memref, %v: memref) { // 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 + omp.atomic.read %v = %x hint(speculative) hint(contended) : memref + return +} + +// ----- + +func @omp_atomic_read6(%x: memref, %v: memref) { + // expected-error @below {{read and write must not be to the same location for atomic reads}} + omp.atomic.read %x = %x hint(speculative) : memref return } diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir --- a/mlir/test/Dialect/OpenMP/ops.mlir +++ b/mlir/test/Dialect/OpenMP/ops.mlir @@ -491,20 +491,20 @@ } // CHECK-LABEL: omp_atomic_read -// CHECK-SAME: (%[[ADDR:.*]]: memref) -func @omp_atomic_read(%addr : memref) { - // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] : memref -> i32 - %1 = omp.atomic.read %addr : memref -> i32 - // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] memory_order(seq_cst) : memref -> i32 - %2 = omp.atomic.read %addr memory_order(seq_cst) : memref -> i32 - // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] memory_order(acquire) : memref -> i32 - %5 = omp.atomic.read %addr memory_order(acquire) : memref -> i32 - // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] memory_order(relaxed) : memref -> i32 - %6 = omp.atomic.read %addr memory_order(relaxed) : memref -> i32 - // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] hint(contended, nonspeculative) : memref -> i32 - %7 = omp.atomic.read %addr hint(nonspeculative, contended) : memref -> i32 - // CHECK: %{{.*}} = omp.atomic.read %[[ADDR]] memory_order(seq_cst) hint(contended, speculative) : memref -> i32 - %8 = omp.atomic.read %addr hint(speculative, contended) memory_order(seq_cst) : memref -> i32 +// CHECK-SAME: (%[[v:.*]]: memref, %[[x:.*]]: memref) +func @omp_atomic_read(%v: memref, %x: memref) { + // CHECK: omp.atomic.read %[[v]] = %[[x]] : memref + omp.atomic.read %v = %x : memref + // CHECK: omp.atomic.read %[[v]] = %[[x]] memory_order(seq_cst) : memref + omp.atomic.read %v = %x memory_order(seq_cst) : memref + // CHECK: omp.atomic.read %[[v]] = %[[x]] memory_order(acquire) : memref + omp.atomic.read %v = %x memory_order(acquire) : memref + // CHECK: omp.atomic.read %[[v]] = %[[x]] memory_order(relaxed) : memref + omp.atomic.read %v = %x memory_order(relaxed) : memref + // CHECK: omp.atomic.read %[[v]] = %[[x]] hint(contended, nonspeculative) : memref + omp.atomic.read %v = %x hint(nonspeculative, contended) : memref + // CHECK: omp.atomic.read %[[v]] = %[[x]] memory_order(seq_cst) hint(contended, speculative) : memref + omp.atomic.read %v = %x hint(speculative, contended) memory_order(seq_cst) : memref return } diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir --- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir +++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir @@ -710,30 +710,26 @@ // ----- // CHECK-LABEL: @omp_atomic_read -// CHECK-SAME: (i32* %[[ARG0:.*]]) -llvm.func @omp_atomic_read(%arg0 : !llvm.ptr) -> () { - // CHECK: %{{.*}} = alloca i32, align 4 - // CHECK: %{{.*}} = alloca i32, align 4 - // CHECK: %{{.*}} = alloca i32, align 4 - // CHECK: %{{.*}} = alloca i32, align 4 +// CHECK-SAME: (i32* %[[ARG0:.*]], i32* %[[ARG1:.*]]) +llvm.func @omp_atomic_read(%arg0 : !llvm.ptr, %arg1 : !llvm.ptr) -> () { // CHECK: %[[X1:.*]] = load atomic i32, i32* %[[ARG0]] monotonic, align 4 - // CHECK: store i32 %[[X1]], i32* %{{.*}}, align 4 - %x1 = omp.atomic.read %arg0 : !llvm.ptr -> i32 + // CHECK: store i32 %[[X1]], i32* %[[ARG1]], align 4 + omp.atomic.read %arg1 = %arg0 : !llvm.ptr // CHECK: %[[X2:.*]] = load atomic i32, i32* %[[ARG0]] seq_cst, align 4 // CHECK: call void @__kmpc_flush(%{{.*}}) - // CHECK: store i32 %[[X2]], i32* %{{.*}}, align 4 - %x2 = omp.atomic.read %arg0 memory_order(seq_cst) : !llvm.ptr -> i32 + // CHECK: store i32 %[[X2]], i32* %[[ARG1]], align 4 + omp.atomic.read %arg1 = %arg0 memory_order(seq_cst) : !llvm.ptr // CHECK: %[[X3:.*]] = load atomic i32, i32* %[[ARG0]] acquire, align 4 // CHECK: call void @__kmpc_flush(%{{.*}}) - // CHECK: store i32 %[[X3]], i32* %{{.*}}, align 4 - %x3 = omp.atomic.read %arg0 memory_order(acquire) : !llvm.ptr -> i32 + // CHECK: store i32 %[[X3]], i32* %[[ARG1]], align 4 + omp.atomic.read %arg1 = %arg0 memory_order(acquire) : !llvm.ptr // CHECK: %[[X4:.*]] = load atomic i32, i32* %[[ARG0]] monotonic, align 4 - // CHECK: store i32 %[[X4]], i32* %{{.*}}, align 4 - %x4 = omp.atomic.read %arg0 memory_order(relaxed) : !llvm.ptr -> i32 + // CHECK: store i32 %[[X4]], i32* %[[ARG1]], align 4 + omp.atomic.read %arg1 = %arg0 memory_order(relaxed) : !llvm.ptr llvm.return }