[Mlir-commits] [mlir] [mlir][DataLayout] Add `IsolatedFromAbove` to `DataLayoutOpInterface` (PR #132742)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Fabian Mora (fabianmcg)

<details>
<summary>Changes</summary>

This patch adds the `IsolatedFromAbove` trait as a dependent trait to the `DataLayoutOpInterface` op interface.

The motivation behind this change comes from the implementation of the `ptr` dialect, specifically the `ptr.type_offset` op.  This op produces an int-like value that equates to the size of a memory element. This is useful for ptr arithmetic and indexing arrays. For example:

```mlir
%f32_off = ptr.type_offset f32 : index
%addr = ptr.ptradd %ptr, %f32_off : !ptr, index
%x = ptr.load %addr : !ptr -> f32 // Read ptr[1]
```

Without the `IsolatedFromAvobe` trait in the DL interface, the `ptr.type_offset` cannot be `ConstantLike`. Why?
Take the example:
```mlir
op {DL1} {
  %f32_off0 = ptr.type_offset f32 : index
  op {DL2} {
    %f32_off1 = ptr.type_offset f32 : index
  }
}
```
If `ptr.type_offset` were to be `ConstantLike` then `canonicalize` would hoist and unique the value. However, that would be wrong as DL2 can have an entry to specify the size that's different from the size in DL1.

The best solution to the above problem is to make `DataLayoutOpInterface` require the `IsolatedFromAbove` trait, as it preserves the constness of values in the DL with respect to the canonicalizer.


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


3 Files Affected:

- (modified) mlir/include/mlir/Dialect/GPU/IR/GPUOps.td (+1-1) 
- (modified) mlir/include/mlir/Interfaces/DataLayoutInterfaces.td (+1-1) 
- (modified) mlir/test/lib/Dialect/Test/TestOps.td (+1-1) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/GPU/IR/GPUOps.td b/mlir/include/mlir/Dialect/GPU/IR/GPUOps.td
index 2b1ce573effd0..3241aff4b683c 100644
--- a/mlir/include/mlir/Dialect/GPU/IR/GPUOps.td
+++ b/mlir/include/mlir/Dialect/GPU/IR/GPUOps.td
@@ -1388,7 +1388,7 @@ def GPU_BarrierOp : GPU_Op<"barrier"> {
 }
 
 def GPU_GPUModuleOp : GPU_Op<"module", [
-      DataLayoutOpInterface, HasDefaultDLTIDataLayout, IsolatedFromAbove,
+      IsolatedFromAbove, DataLayoutOpInterface, HasDefaultDLTIDataLayout,
       NoRegionArguments, SymbolTable, Symbol] # GraphRegionNoTerminator.traits> {
   let summary = "A top level compilation unit containing code to be run on a GPU.";
   let description = [{
diff --git a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
index 818b441b9a770..fc701b20cb007 100644
--- a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
+++ b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
@@ -565,7 +565,7 @@ def DataLayoutOpInterface : OpInterface<"DataLayoutOpInterface"> {
       }]
     >
   ];
-
+  let dependentTraits = [IsolatedFromAbove];
   let verify = [{ return ::mlir::detail::verifyDataLayoutOp($_op); }];
 }
 
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 94c722038f1cc..f653e4465cfef 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -2555,7 +2555,7 @@ def MakeTupleOp: TEST_Op<"make_tuple"> {
 //===----------------------------------------------------------------------===//
 
 def OpWithDataLayoutOp : TEST_Op<"op_with_data_layout",
-                                 [HasDefaultDLTIDataLayout, DataLayoutOpInterface]> {
+                                 [IsolatedFromAbove, HasDefaultDLTIDataLayout, DataLayoutOpInterface]> {
   let summary =
       "An op that uses DataLayout implementation from the Target dialect";
   let regions = (region VariadicRegion<AnyRegion>:$regions);

``````````

</details>


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