[Mlir-commits] [mlir] [mlir][bufferization][NFC] Remove `BufferPlacementTransformationBase` (PR #77317)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-bufferization

Author: Matthias Springer (matthias-springer)

<details>
<summary>Changes</summary>

`BufferPlacementTransformationBase` makes it difficult to add error handling to `BufferViewFlowAnalysis`. Error handling is added to `BufferViewFlowAnalysis` in a subsequent commit.

---
Full diff: https://github.com/llvm/llvm-project/pull/77317.diff


4 Files Affected:

- (modified) mlir/include/mlir/Dialect/Bufferization/Transforms/BufferUtils.h (-20) 
- (modified) mlir/lib/Dialect/Bufferization/Transforms/BufferDeallocation.cpp (+14-4) 
- (modified) mlir/lib/Dialect/Bufferization/Transforms/BufferOptimizations.cpp (+25-6) 
- (modified) mlir/lib/Dialect/Bufferization/Transforms/BufferUtils.cpp (-9) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/Bufferization/Transforms/BufferUtils.h b/mlir/include/mlir/Dialect/Bufferization/Transforms/BufferUtils.h
index e5f3b6d571f437..2b212815eedb3d 100644
--- a/mlir/include/mlir/Dialect/Bufferization/Transforms/BufferUtils.h
+++ b/mlir/include/mlir/Dialect/Bufferization/Transforms/BufferUtils.h
@@ -98,26 +98,6 @@ Block *findCommonDominator(Value value,
   return doms.findNearestCommonDominator(blocks);
 }
 
-/// The base class for all BufferPlacement transformations.
-class BufferPlacementTransformationBase {
-public:
-  using ValueSetT = BufferViewFlowAnalysis::ValueSetT;
-
-  /// Constructs a new operation base using the given root operation.
-  BufferPlacementTransformationBase(Operation *op);
-
-protected:
-  /// Alias information that can be updated during the insertion of copies.
-  BufferViewFlowAnalysis aliases;
-
-  /// Stores all internally managed allocations.
-  BufferPlacementAllocs allocs;
-
-  /// The underlying liveness analysis to compute fine grained information
-  /// about alloc and dealloc positions.
-  Liveness liveness;
-};
-
 // Create a global op for the given tensor-valued constant in the program.
 // Globals are created lazily at the top of the enclosing ModuleOp with pretty
 // names. Duplicates are avoided.
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferDeallocation.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferDeallocation.cpp
index a0a81d4add7121..374982d5315dd7 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferDeallocation.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferDeallocation.cpp
@@ -201,14 +201,14 @@ class Backedges {
 /// The buffer deallocation transformation which ensures that all allocs in the
 /// program have a corresponding de-allocation. As a side-effect, it might also
 /// introduce clones that in turn leads to additional deallocations.
-class BufferDeallocation : public BufferPlacementTransformationBase {
+class BufferDeallocation {
 public:
   using AliasAllocationMapT =
       llvm::DenseMap<Value, bufferization::AllocationOpInterface>;
 
   BufferDeallocation(Operation *op)
-      : BufferPlacementTransformationBase(op), dominators(op),
-        postDominators(op) {}
+      : dominators(op), postDominators(op), aliases(op), allocs(op),
+        liveness(op) {}
 
   /// Checks if all allocation operations either provide an already existing
   /// deallocation operation or implement the AllocationOpInterface. In
@@ -615,10 +615,20 @@ class BufferDeallocation : public BufferPlacementTransformationBase {
   PostDominanceInfo postDominators;
 
   /// Stores already cloned buffers to avoid additional clones of clones.
-  ValueSetT clonedValues;
+  BufferViewFlowAnalysis::ValueSetT clonedValues;
 
   /// Maps aliases to their source allocation interfaces (inverse mapping).
   AliasAllocationMapT aliasToAllocations;
+
+  /// Alias information that can be updated during the insertion of copies.
+  BufferViewFlowAnalysis aliases;
+
+  /// Stores all internally managed allocations.
+  BufferPlacementAllocs allocs;
+
+  /// The underlying liveness analysis to compute fine grained information
+  /// about alloc and dealloc positions.
+  Liveness liveness;
 };
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferOptimizations.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferOptimizations.cpp
index 9dc2f262a51161..47819284a4cf8c 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferOptimizations.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferOptimizations.cpp
@@ -166,12 +166,11 @@ struct BufferAllocationHoistingStateBase {
 };
 
 /// Implements the actual hoisting logic for allocation nodes.
-template <typename StateT>
-class BufferAllocationHoisting : public BufferPlacementTransformationBase {
+template <typename StateT> class BufferAllocationHoisting {
 public:
   BufferAllocationHoisting(Operation *op)
-      : BufferPlacementTransformationBase(op), dominators(op),
-        postDominators(op), scopeOp(op) {}
+      : dominators(op), postDominators(op), scopeOp(op), aliases(op),
+        allocs(op), liveness(op) {}
 
   /// Moves allocations upwards.
   void hoist() {
@@ -285,6 +284,16 @@ class BufferAllocationHoisting : public BufferPlacementTransformationBase {
   /// The operation that this transformation is working on. It is used to also
   /// gather allocas.
   Operation *scopeOp;
+
+  /// Alias information that can be updated during the insertion of copies.
+  BufferViewFlowAnalysis aliases;
+
+  /// Stores all internally managed allocations.
+  BufferPlacementAllocs allocs;
+
+  /// The underlying liveness analysis to compute fine grained information
+  /// about alloc and dealloc positions.
+  Liveness liveness;
 };
 
 /// A state implementation compatible with the `BufferAllocationHoisting` class
@@ -365,10 +374,10 @@ struct BufferAllocationLoopHoistingState : BufferAllocationHoistingStateBase {
 //===----------------------------------------------------------------------===//
 
 /// Promotes heap-based allocations to stack-based allocations (if possible).
-class BufferPlacementPromotion : BufferPlacementTransformationBase {
+class BufferPlacementPromotion {
 public:
   BufferPlacementPromotion(Operation *op)
-      : BufferPlacementTransformationBase(op) {}
+      : aliases(op), allocs(op), liveness(op) {}
 
   /// Promote buffers to stack-based allocations.
   void promote(function_ref<bool(Value)> isSmallAlloc) {
@@ -400,6 +409,16 @@ class BufferPlacementPromotion : BufferPlacementTransformationBase {
       }
     }
   }
+
+  /// Alias information that can be updated during the insertion of copies.
+  BufferViewFlowAnalysis aliases;
+
+  /// Stores all internally managed allocations.
+  BufferPlacementAllocs allocs;
+
+  /// The underlying liveness analysis to compute fine grained information
+  /// about alloc and dealloc positions.
+  Liveness liveness;
 };
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferUtils.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferUtils.cpp
index 8fffdbf664c3f4..ee4bd4d9ec4aac 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferUtils.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferUtils.cpp
@@ -91,15 +91,6 @@ void BufferPlacementAllocs::build(Operation *op) {
 // BufferPlacementTransformationBase
 //===----------------------------------------------------------------------===//
 
-/// Constructs a new transformation base using the given root operation.
-BufferPlacementTransformationBase::BufferPlacementTransformationBase(
-    Operation *op)
-    : aliases(op), allocs(op), liveness(op) {}
-
-//===----------------------------------------------------------------------===//
-// BufferPlacementTransformationBase
-//===----------------------------------------------------------------------===//
-
 FailureOr<memref::GlobalOp>
 bufferization::getGlobalFor(arith::ConstantOp constantOp, uint64_t alignment,
                             Attribute memorySpace) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/77317