https://github.com/zero9178 updated https://github.com/llvm/llvm-project/pull/65730:
From d32d6c5faca4141120482b7346c75f6656a29299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.bock+l...@nextsilicon.com> Date: Fri, 8 Sep 2023 11:16:22 +0200 Subject: [PATCH 1/2] [mlir][LLVM] Model side effects of volatile and atomic load-store According to the LLVM language reference, both volatile memory operations and atomic operations (except unordered) do not simply read memory but also perform write operations on arbitrary memory[0][1]. In the case of volatile memory operations, this is the case due to the read possibly having target specific properties. A common real-world situation where this happens is reading memory mapped registers on an MCU for example. Atomic operations are more special. They form a kind of memory barrier which from the perspective of the optimizer/lang-ref makes writes from other threads visible in the current thread. Any kind of synchronization can therefore conservatively be modeled as a write-effect. This PR therefore adjusts the side effects of `llvm.load` and `llvm.store` to add unknown global read and write effects if they are either atomic or volatile. Regarding testing: I am not sure how to best test this change for `llvm.store` and the "globalness" of the effect that isn't just a unit test checking that the output matches exactly. For the time being, I added a test making sure that `llvm.load` does not get DCEd in aforementioned cases. Related logic in LLVM proper: https://github.com/llvm/llvm-project/blob/3398744a6106c83993611bd3c5e79ec6b94417dc/llvm/lib/IR/Instruction.cpp#L638-L676 https://github.com/llvm/llvm-project/blob/3398744a6106c83993611bd3c5e79ec6b94417dc/llvm/include/llvm/IR/Instructions.h#L258-L262 [0] https://llvm.org/docs/LangRef.html#volatile-memory-accesses [1] https://llvm.org/docs/Atomics.html#monotonic --- mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 10 ++++++---- mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 22 +++++++++++++++++++++ mlir/test/Dialect/LLVMIR/canonicalize.mlir | 17 ++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td index 7ceec72144eb52c..e9ed7f1186ee58a 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td @@ -326,9 +326,10 @@ def LLVM_GEPOp : LLVM_Op<"getelementptr", [Pure, } def LLVM_LoadOp : LLVM_MemAccessOpBase<"load", - [DeclareOpInterfaceMethods<PromotableMemOpInterface>, + [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>, + DeclareOpInterfaceMethods<PromotableMemOpInterface>, DeclareOpInterfaceMethods<SafeMemorySlotAccessOpInterface>]> { - dag args = (ins Arg<LLVM_PointerTo<LLVM_LoadableType>, "", [MemRead]>:$addr, + dag args = (ins LLVM_PointerTo<LLVM_LoadableType>:$addr, OptionalAttr<I64Attr>:$alignment, UnitAttr:$volatile_, UnitAttr:$nontemporal, @@ -399,10 +400,11 @@ def LLVM_LoadOp : LLVM_MemAccessOpBase<"load", } def LLVM_StoreOp : LLVM_MemAccessOpBase<"store", - [DeclareOpInterfaceMethods<PromotableMemOpInterface>, + [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>, + DeclareOpInterfaceMethods<PromotableMemOpInterface>, DeclareOpInterfaceMethods<SafeMemorySlotAccessOpInterface>]> { dag args = (ins LLVM_LoadableType:$value, - Arg<LLVM_PointerTo<LLVM_LoadableType>,"",[MemWrite]>:$addr, + LLVM_PointerTo<LLVM_LoadableType>:$addr, OptionalAttr<I64Attr>:$alignment, UnitAttr:$volatile_, UnitAttr:$nontemporal, diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp index ae01f7c4621522b..a4e5c25f3394e67 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -812,6 +812,17 @@ Type GEPOp::getResultPtrElementType() { // LoadOp //===----------------------------------------------------------------------===// +void LoadOp::getEffects( + SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>> + &effects) { + effects.emplace_back(MemoryEffects::Read::get(), getAddr()); + if (getVolatile_() || (getOrdering() != AtomicOrdering::not_atomic && + getOrdering() != AtomicOrdering::unordered)) { + effects.emplace_back(MemoryEffects::Write::get()); + effects.emplace_back(MemoryEffects::Read::get()); + } +} + /// Returns true if the given type is supported by atomic operations. All /// integer and float types with limited bit width are supported. Additionally, /// depending on the operation pointers may be supported as well. @@ -932,6 +943,17 @@ static void printLoadType(OpAsmPrinter &printer, Operation *op, Type type, // StoreOp //===----------------------------------------------------------------------===// +void StoreOp::getEffects( + SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>> + &effects) { + effects.emplace_back(MemoryEffects::Write::get(), getAddr()); + if (getVolatile_() || (getOrdering() != AtomicOrdering::not_atomic && + getOrdering() != AtomicOrdering::unordered)) { + effects.emplace_back(MemoryEffects::Write::get()); + effects.emplace_back(MemoryEffects::Read::get()); + } +} + LogicalResult StoreOp::verify() { Type valueType = getValue().getType(); return verifyAtomicMemOp(*this, valueType, diff --git a/mlir/test/Dialect/LLVMIR/canonicalize.mlir b/mlir/test/Dialect/LLVMIR/canonicalize.mlir index 3e7f689bdc03e31..ed7efabb44b1ac3 100644 --- a/mlir/test/Dialect/LLVMIR/canonicalize.mlir +++ b/mlir/test/Dialect/LLVMIR/canonicalize.mlir @@ -191,3 +191,20 @@ llvm.func @alloca_dce() { %0 = llvm.alloca %c1_i64 x i32 : (i64) -> !llvm.ptr llvm.return } + +// ----- + +// CHECK-LABEL: func @volatile_load +llvm.func @volatile_load(%x : !llvm.ptr) { + // A volatile load may have side-effects such as a write operation to arbitrary memory. + // Make sure it is not removed. + // CHECK: llvm.load volatile + %0 = llvm.load volatile %x : !llvm.ptr -> i8 + // Same with monotonic atomics and any stricter modes. + // CHECK: llvm.load %{{.*}} atomic monotonic + %2 = llvm.load %x atomic monotonic { alignment = 1 } : !llvm.ptr -> i8 + // But not unordered! + // CHECK-NOT: llvm.load %{{.*}} atomic unordered + %3 = llvm.load %x atomic unordered { alignment = 1 } : !llvm.ptr -> i8 + llvm.return +} From 8e056d5baad0c35880b8337064f9778c03ccc22b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.bock+l...@nextsilicon.com> Date: Fri, 8 Sep 2023 13:10:32 +0200 Subject: [PATCH 2/2] address review comments --- mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp index a4e5c25f3394e67..c3575d299b3888a 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -816,6 +816,11 @@ void LoadOp::getEffects( SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>> &effects) { effects.emplace_back(MemoryEffects::Read::get(), getAddr()); + // Volatile operations can have target-specific read-write effects on + // memory besides the one referred to by the pointer operand. + // Similarly, atomic operations that are monotonic or stricter cause + // synchronization that from a language point-of-view, are arbitrary + // read-writes into memory. if (getVolatile_() || (getOrdering() != AtomicOrdering::not_atomic && getOrdering() != AtomicOrdering::unordered)) { effects.emplace_back(MemoryEffects::Write::get()); @@ -947,6 +952,11 @@ void StoreOp::getEffects( SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>> &effects) { effects.emplace_back(MemoryEffects::Write::get(), getAddr()); + // Volatile operations can have target-specific read-write effects on + // memory besides the one referred to by the pointer operand. + // Similarly, atomic operations that are monotonic or stricter cause + // synchronization that from a language point-of-view, are arbitrary + // read-writes into memory. if (getVolatile_() || (getOrdering() != AtomicOrdering::not_atomic && getOrdering() != AtomicOrdering::unordered)) { effects.emplace_back(MemoryEffects::Write::get()); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
