[Mlir-commits] [mlir] feb7bea - [mlir][LLVM] Model side effects of volatile and atomic load-store (#65730)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Sep 8 04:50:53 PDT 2023
Author: Markus Böck
Date: 2023-09-08T13:50:49+02:00
New Revision: feb7beaf70bace1c3ffafc7f732c8fadca5e8c9d
URL: https://github.com/llvm/llvm-project/commit/feb7beaf70bace1c3ffafc7f732c8fadca5e8c9d
DIFF: https://github.com/llvm/llvm-project/commit/feb7beaf70bace1c3ffafc7f732c8fadca5e8c9d.diff
LOG: [mlir][LLVM] Model side effects of volatile and atomic load-store (#65730)
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
Added:
Modified:
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
mlir/test/Dialect/LLVMIR/canonicalize.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 7c03e0bd4451aed..57dce72e102e7a4 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..c3575d299b3888a 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -812,6 +812,22 @@ Type GEPOp::getResultPtrElementType() {
// LoadOp
//===----------------------------------------------------------------------===//
+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());
+ 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 +948,22 @@ 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());
+ // 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());
+ 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
+}
More information about the Mlir-commits
mailing list