[Mlir-commits] [mlir] 35f8945 - [mlir] Made DefaultResource the root of memory resource hierarchy. (#187423)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon Mar 30 17:52:50 PDT 2026
Author: Slava Zakharin
Date: 2026-03-30T17:52:45-07:00
New Revision: 35f89458fa2871b7590c115c9afea764dfc11c02
URL: https://github.com/llvm/llvm-project/commit/35f89458fa2871b7590c115c9afea764dfc11c02
DIFF: https://github.com/llvm/llvm-project/commit/35f89458fa2871b7590c115c9afea764dfc11c02.diff
LOG: [mlir] Made DefaultResource the root of memory resource hierarchy. (#187423)
DefaultResource is made the root of the memory resource hierarchy,
so now it overlaps with all resources.
RFC:
https://discourse.llvm.org/t/rfc-mlir-memory-region-hierarchy-for-mlir-side-effects/89811/32
Added:
Modified:
mlir/include/mlir/Interfaces/SideEffectInterfaces.h
mlir/lib/Transforms/CSE.cpp
mlir/test/Analysis/test-alias-analysis-modref.mlir
mlir/test/IR/test-side-effects.mlir
mlir/test/Transforms/cse.mlir
mlir/test/lib/Dialect/Test/TestOpDefs.cpp
mlir/test/lib/Dialect/Test/TestOps.td
mlir/unittests/Interfaces/SideEffectInterfacesTest.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/Interfaces/SideEffectInterfaces.h b/mlir/include/mlir/Interfaces/SideEffectInterfaces.h
index 97f860565b183..dfffbcaf1bbab 100644
--- a/mlir/include/mlir/Interfaces/SideEffectInterfaces.h
+++ b/mlir/include/mlir/Interfaces/SideEffectInterfaces.h
@@ -127,8 +127,8 @@ class Resource {
/// Return a string name of the resource.
virtual StringRef getName() const = 0;
- /// Return the parent resource in the hierarchy, or nullptr for a root.
- virtual Resource *getParent() const { return nullptr; }
+ /// Return the parent resource in the hierarchy.
+ virtual Resource *getParent() const;
/// Returns true if this resource is addressable (effects on it can alias
/// pointer-based memory). Default is true.
@@ -175,10 +175,13 @@ class Resource {
TypeID id;
};
-/// A conservative default resource kind.
+/// The default resource kind. It serves as the root of the resource hierarchy:
+/// all resources that do not override getParent() have DefaultResource as their
+/// parent.
struct DefaultResource : public Resource::Base<DefaultResource> {
DefaultResource() = default;
StringRef getName() const override { return "<Default>"; }
+ Resource *getParent() const override { return nullptr; }
protected:
/// For use when this type is the parent of another resource; allows the
@@ -186,6 +189,10 @@ struct DefaultResource : public Resource::Base<DefaultResource> {
DefaultResource(TypeID id) : Base(id) {}
};
+/// All resources that do not override getParent() have DefaultResource
+/// as their parent.
+inline Resource *Resource::getParent() const { return DefaultResource::get(); }
+
/// An automatic allocation-scope resource that is valid in the context of a
/// parent AutomaticAllocationScope trait.
struct AutomaticAllocationScopeResource
diff --git a/mlir/lib/Transforms/CSE.cpp b/mlir/lib/Transforms/CSE.cpp
index 7870f1f401890..289cfb8408c37 100644
--- a/mlir/lib/Transforms/CSE.cpp
+++ b/mlir/lib/Transforms/CSE.cpp
@@ -182,15 +182,15 @@ bool CSEDriver::hasOtherSideEffectingOpInBetween(Operation *fromOp,
assert(hasEffect<MemoryEffects::Read>(toOp) &&
"expected read effect on toOp");
- // Collect the resources of fromOp's read effects. A write can only block
- // CSE if its resource is not disjoint from one of these.
- SmallPtrSet<SideEffects::Resource *, 1> readResources;
+ // Collect the read effects of fromOp. A write can only block CSE if it
+ // can conflict with one of these reads.
+ SmallVector<MemoryEffects::EffectInstance> readEffects;
if (auto memOp = dyn_cast<MemoryEffectOpInterface>(fromOp)) {
SmallVector<MemoryEffects::EffectInstance> fromEffects;
memOp.getEffects(fromEffects);
- for (const auto &e : fromEffects)
+ for (MemoryEffects::EffectInstance &e : fromEffects)
if (isa<MemoryEffects::Read>(e.getEffect()))
- readResources.insert(e.getResource());
+ readEffects.push_back(e);
}
Operation *nextOp = fromOp->getNextNode();
@@ -224,10 +224,20 @@ bool CSEDriver::hasOtherSideEffectingOpInBetween(Operation *fromOp,
if (isa<MemoryEffects::Write>(effect.getEffect())) {
// A write on a resource disjoint from all read resources cannot
// conflict with the reads being CSE'd.
- auto *writeResource = effect.getResource();
- bool canConflict = llvm::any_of(readResources, [&](auto *readResource) {
- return !writeResource->isDisjointFrom(readResource);
- });
+ SideEffects::Resource *writeResource = effect.getResource();
+ bool canConflict =
+ llvm::any_of(readEffects, [&](const auto &readEffect) {
+ SideEffects::Resource *readResource = readEffect.getResource();
+ if (writeResource->isDisjointFrom(readResource))
+ return false;
+ // A pointer-based access to an addressable resource cannot
+ // conflict with a non-addressable resource.
+ if (readEffect.getValue() && !writeResource->isAddressable())
+ return false;
+ if (effect.getValue() && !readResource->isAddressable())
+ return false;
+ return true;
+ });
if (canConflict) {
result.first->second = {nextOp, MemoryEffects::Write::get()};
return true;
diff --git a/mlir/test/Analysis/test-alias-analysis-modref.mlir b/mlir/test/Analysis/test-alias-analysis-modref.mlir
index b82f789f4d77d..2a9c612317d53 100644
--- a/mlir/test/Analysis/test-alias-analysis-modref.mlir
+++ b/mlir/test/Analysis/test-alias-analysis-modref.mlir
@@ -89,3 +89,26 @@ func.func @addressable_write(%arg: memref<2xf32>) attributes {test.ptr = "func"}
"test.side_effect_op"() {effects = [{effect="write"}], test.ptr = "side_effect_op"} : () -> i32
return {test.ptr = "return"}
}
+
+// -----
+
+// An op with no side effects does not mod-ref any memory.
+// CHECK-LABEL: Testing : "conditional_no_effects"
+// CHECK-DAG: conditional_side_effect_op -> func.region0#0: NoModRef
+// CHECK-DAG: return -> func.region0#0: NoModRef
+func.func @conditional_no_effects(%arg: memref<2xf32>) attributes {test.ptr = "func"} {
+ "test.conditional_side_effect_op"() {has_effects = false, test.ptr = "conditional_side_effect_op"} : () -> i32
+ return {test.ptr = "return"}
+}
+
+// -----
+
+// An op with read/write/free/alloc effects on DefaultResource
+// may mod-ref any memory.
+// CHECK-LABEL: Testing : "conditional_all_effects"
+// CHECK-DAG: conditional_side_effect_op -> func.region0#0: ModRef
+// CHECK-DAG: return -> func.region0#0: NoModRef
+func.func @conditional_all_effects(%arg: memref<2xf32>) attributes {test.ptr = "func"} {
+ "test.conditional_side_effect_op"() {has_effects = true, test.ptr = "conditional_side_effect_op"} : () -> i32
+ return {test.ptr = "return"}
+}
diff --git a/mlir/test/IR/test-side-effects.mlir b/mlir/test/IR/test-side-effects.mlir
index b652ecb7dad1d..f66937a9ec943 100644
--- a/mlir/test/IR/test-side-effects.mlir
+++ b/mlir/test/IR/test-side-effects.mlir
@@ -56,5 +56,14 @@ func.func @side_effect(%arg : index) {
test.region_yield %arg0 : index
} {effects = [ {effect="allocate", on_argument, test_resource} ]} : index -> index
+ // expected-remark at +1 {{operation has no memory effects}}
+ %9 = "test.conditional_side_effect_op"() {has_effects = false} : () -> i32
+
+ // expected-remark at +4 {{found an instance of 'read' on resource '<Default>'}}
+ // expected-remark at +3 {{found an instance of 'write' on resource '<Default>'}}
+ // expected-remark at +2 {{found an instance of 'free' on resource '<Default>'}}
+ // expected-remark at +1 {{found an instance of 'allocate' on resource '<Default>'}}
+ %10 = "test.conditional_side_effect_op"() {has_effects = true} : () -> i32
+
func.return
}
diff --git a/mlir/test/Transforms/cse.mlir b/mlir/test/Transforms/cse.mlir
index 14cdcdeffd1ec..4b2907287d89e 100644
--- a/mlir/test/Transforms/cse.mlir
+++ b/mlir/test/Transforms/cse.mlir
@@ -576,14 +576,30 @@ func.func @cse_recursive_effects_failure() -> (i32, i32, i32) {
// -----
-/// Check that a write on a resource disjoint from DefaultResource does not
-/// block CSE of reads on DefaultResource, because the resources are disjoint.
-// CHECK-LABEL: @cse_non_addressable_write_does_not_block
-func.func @cse_non_addressable_write_does_not_block() -> i32 {
- // CHECK-NEXT: %[[V:.*]] = "test.op_with_memread"() : () -> i32
+/// A non-addressable write on a subresource of DefaultResource, so it is NOT
+/// disjoint. Without a pointer-based read, the write conservatively conflicts.
+// CHECK-LABEL: @cse_non_addressable_write_blocks_non_pointer_read
+func.func @cse_non_addressable_write_blocks_non_pointer_read() -> i32 {
+ // CHECK-NEXT: %[[V0:.*]] = "test.op_with_memread"() : () -> i32
%0 = "test.op_with_memread"() : () -> (i32)
"test.side_effect_op"() {effects = [{effect="write", test_nonaddressable_resource}]} : () -> i32
+ // CHECK: %[[V1:.*]] = "test.op_with_memread"() : () -> i32
%1 = "test.op_with_memread"() : () -> (i32)
+ // CHECK: %{{.*}} = arith.addi %[[V0]], %[[V1]] : i32
+ %2 = arith.addi %0, %1 : i32
+ return %2 : i32
+}
+
+// -----
+
+/// A pointer-based read on an addressable resource (DefaultResource) cannot
+/// conflict with a write on a non-addressable resource, so CSE proceeds.
+// CHECK-LABEL: @cse_non_addressable_write_does_not_block_pointer_read
+func.func @cse_non_addressable_write_does_not_block_pointer_read() -> i32 {
+ // CHECK-NEXT: %[[V:.*]] = "test.side_effect_op"()
+ %0 = "test.side_effect_op"() {effects = [{effect="read", on_result}]} : () -> i32
+ "test.side_effect_op"() {effects = [{effect="write", test_nonaddressable_resource}]} : () -> i32
+ %1 = "test.side_effect_op"() {effects = [{effect="read", on_result}]} : () -> i32
// CHECK-NEXT: %{{.*}} = "test.side_effect_op"()
// CHECK-NEXT: %{{.*}} = arith.addi %[[V]], %[[V]] : i32
%2 = arith.addi %0, %1 : i32
@@ -592,7 +608,7 @@ func.func @cse_non_addressable_write_does_not_block() -> i32 {
// -----
-/// Check that consecutive reads on the same resource (disjoint from DefaultResource) are CSE'd
+/// Check that consecutive reads on the same non-addressable resource are CSE'd
/// when there is no intervening write.
// CHECK-LABEL: @cse_reads_on_same_nonaddressable_resource
func.func @cse_reads_on_same_nonaddressable_resource() -> i32 {
@@ -623,7 +639,7 @@ func.func @cse_write_same_nonaddressable_resource_blocks() -> i32 {
// -----
/// Check that a write to a resource disjoint from the read resource does NOT
-/// block CSE (sibling resources under
diff erent roots are disjoint).
+/// block CSE (sibling non-addressable subresources are disjoint).
// CHECK-LABEL: @cse_write_disjoint_nonaddressable_subresource_allows
func.func @cse_write_disjoint_nonaddressable_subresource_allows() -> i32 {
// CHECK-NEXT: %[[V:.*]] = "test.side_effect_op"()
@@ -651,3 +667,19 @@ func.func @cse_addressable_custom_resource_write_blocks() -> i32 {
%2 = arith.addi %0, %1 : i32
return %2 : i32
}
+
+// -----
+
+/// A pointer-based write on an addressable resource (DefaultResource) cannot
+/// conflict with a read on a non-addressable resource, so CSE proceeds.
+// CHECK-LABEL: @cse_pointer_write_does_not_block_non_addressable_read
+func.func @cse_pointer_write_does_not_block_non_addressable_read() -> i32 {
+ // CHECK-NEXT: %[[V:.*]] = "test.side_effect_op"()
+ %0 = "test.side_effect_op"() {effects = [{effect="read", test_nonaddressable_resource}]} : () -> i32
+ "test.side_effect_op"() {effects = [{effect="write", on_result}]} : () -> i32
+ %1 = "test.side_effect_op"() {effects = [{effect="read", test_nonaddressable_resource}]} : () -> i32
+ // CHECK-NEXT: %{{.*}} = "test.side_effect_op"()
+ // CHECK-NEXT: %{{.*}} = arith.addi %[[V]], %[[V]] : i32
+ %2 = arith.addi %0, %1 : i32
+ return %2 : i32
+}
diff --git a/mlir/test/lib/Dialect/Test/TestOpDefs.cpp b/mlir/test/lib/Dialect/Test/TestOpDefs.cpp
index ed5dc5bead78a..5d2121ce26671 100644
--- a/mlir/test/lib/Dialect/Test/TestOpDefs.cpp
+++ b/mlir/test/lib/Dialect/Test/TestOpDefs.cpp
@@ -501,6 +501,22 @@ void SideEffectOp::getEffects(
testSideEffectOpGetEffect(getOperation(), effects);
}
+//===----------------------------------------------------------------------===//
+// ConditionalSideEffectOp
+//===----------------------------------------------------------------------===//
+
+void ConditionalSideEffectOp::getEffects(
+ SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+ if (!getHasEffects())
+ return;
+
+ SideEffects::Resource *resource = SideEffects::DefaultResource::get();
+ effects.emplace_back(MemoryEffects::Read::get(), resource);
+ effects.emplace_back(MemoryEffects::Write::get(), resource);
+ effects.emplace_back(MemoryEffects::Free::get(), resource);
+ effects.emplace_back(MemoryEffects::Allocate::get(), resource);
+}
+
void SideEffectWithRegionOp::getEffects(
SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
// Check for an effects attribute on the op instance.
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 4c9e6b3fe9e45..f9ffddd1300b6 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -2472,6 +2472,13 @@ def SideEffectOp : TEST_Op<"side_effect_op",
let results = (outs AnyType:$result);
}
+def ConditionalSideEffectOp
+ : TEST_Op<"conditional_side_effect_op", [DeclareOpInterfaceMethods<
+ MemoryEffectsOpInterface>]> {
+ let arguments = (ins BoolAttr:$has_effects);
+ let results = (outs AnyType:$result);
+}
+
def SideEffectWithRegionOp : TEST_Op<"side_effect_with_region_op",
[DeclareOpInterfaceMethods<MemoryEffectsOpInterface>,
DeclareOpInterfaceMethods<TestEffectOpInterface>]> {
diff --git a/mlir/unittests/Interfaces/SideEffectInterfacesTest.cpp b/mlir/unittests/Interfaces/SideEffectInterfacesTest.cpp
index 2f7f7a889ccab..d7dbdd08f0416 100644
--- a/mlir/unittests/Interfaces/SideEffectInterfacesTest.cpp
+++ b/mlir/unittests/Interfaces/SideEffectInterfacesTest.cpp
@@ -91,7 +91,8 @@ TEST(SideEffectResourceTest, CustomHierarchyIsaCast) {
EXPECT_EQ(TestGrandchildResource::get()->getParent(),
TestChildResource::get());
EXPECT_EQ(TestChildResource::get()->getParent(), TestRootResource::get());
- EXPECT_EQ(TestRootResource::get()->getParent(), nullptr);
+ EXPECT_EQ(TestRootResource::get()->getParent(), DefaultResource::get());
+ EXPECT_EQ(DefaultResource::get()->getParent(), nullptr);
// isSubresourceOf
EXPECT_TRUE(
@@ -101,13 +102,14 @@ TEST(SideEffectResourceTest, CustomHierarchyIsaCast) {
EXPECT_FALSE(
TestRootResource::get()->isSubresourceOf(TestGrandchildResource::get()));
- // Custom hierarchy disjoint from DefaultResource
- EXPECT_TRUE(TestRootResource::get()->isDisjointFrom(DefaultResource::get()));
- EXPECT_FALSE(isa<DefaultResource>(TestRootResource::get()));
- EXPECT_FALSE(isa<DefaultResource>(TestChildResource::get()));
- EXPECT_FALSE(isa<DefaultResource>(TestGrandchildResource::get()));
+ // Custom hierarchy is a subresource of DefaultResource (the root).
+ EXPECT_FALSE(TestRootResource::get()->isDisjointFrom(DefaultResource::get()));
+ EXPECT_TRUE(TestRootResource::get()->isSubresourceOf(DefaultResource::get()));
+ EXPECT_TRUE(
+ TestGrandchildResource::get()->isSubresourceOf(DefaultResource::get()));
+ EXPECT_FALSE(
+ DefaultResource::get()->isSubresourceOf(TestRootResource::get()));
EXPECT_FALSE(isa<TestRootResource>(DefaultResource::get()));
- EXPECT_EQ(dyn_cast<DefaultResource>(TestGrandchildResource::get()), nullptr);
EXPECT_EQ(dyn_cast<TestRootResource>(DefaultResource::get()), nullptr);
}
@@ -115,7 +117,10 @@ TEST(SideEffectResourceTest, DisjointnessAndGetParent) {
EXPECT_EQ(DefaultResource::get()->getParent(), nullptr);
EXPECT_EQ(AutomaticAllocationScopeResource::get()->getParent(),
DefaultResource::get());
- EXPECT_TRUE(DefaultResource::get()->isDisjointFrom(TestRootResource::get()));
+ // TestRootResource is a subresource of DefaultResource (the root),
+ // so they are not disjoint.
+ EXPECT_FALSE(DefaultResource::get()->isDisjointFrom(TestRootResource::get()));
+ EXPECT_TRUE(TestRootResource::get()->isSubresourceOf(DefaultResource::get()));
EXPECT_TRUE(
TestChildResource::get()->isSubresourceOf(TestRootResource::get()));
EXPECT_FALSE(
More information about the Mlir-commits
mailing list