[Mlir-commits] [mlir] [MLIR][SideEffects][MemoryEffects] Modified LICM to be more aggressive when checking movability of ops with MemWrite effects (PR #155344)

[Mo Bagherbeik]

================
@@ -1437,3 +1437,338 @@ func.func @do_not_hoist_vector_transfer_ops_memref(
   }
   func.return %final : vector<4x4xf32>
 }
+
+// -----
+
+// CHECK-LABEL func.func @move_single_resource_basic
+func.func @move_single_resource_basic() attributes {} {
+  %c0_i32 = arith.constant 0 : i32
+  %c1_i32 = arith.constant 10 : i32
+  %c2_i32 = arith.constant 1 : i32
+  %c0_i32_0 = arith.constant 0 : i32
+
+  // Single write effect on one resource in a triple-nested loop
+  // No loop-variant inputs to op and no read effects -> movable
+  // CHECK: "test.test_effects_write_A"() : () -> ()
+  // CHECK: scf.for
+  // CHECK: scf.for
+  // CHECK: scf.for
+
+  scf.for %arg0 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+    scf.for %arg1 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+      scf.for %arg2 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+        "test.test_effects_write_A"() : () -> ()
+      }
+    }
+  }
+  return
+}
+
+// -----
+
+// CHECK-LABEL func.func @move_single_resource_write_dominant
+func.func @move_single_resource_write_dominant() attributes {} {
+  %c0_i32 = arith.constant 0 : i32
+  %c1_i32 = arith.constant 10 : i32
+  %c2_i32 = arith.constant 1 : i32
+  %c0_i32_0 = arith.constant 0 : i32
+
+  // Write effect on one resource followed by a Read.
+  // No loop-variant inputs to Write op, no conflict on
+  // "A" --> both ops movable
+  // CHECK: "test.test_effects_write_A"() : () -> ()
+  // CHECK: "test.test_effects_read_A"() : () -> ()
+  // CHECK: scf.for
+  // CHECK: scf.for
+  // CHECK: scf.for
+
+  scf.for %arg0 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+    scf.for %arg1 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+      scf.for %arg2 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+        "test.test_effects_write_A"() : () -> ()
+        "test.test_effects_read_A"() : () -> ()
+      }
+    }
+  }
+  return
+}
+
+// -----
+
+// CHECK-LABEL func.func @move_single_resource_read_dominant
+func.func @move_single_resource_read_dominant() attributes {} {
+  %c0_i32 = arith.constant 0 : i32
+  %c1_i32 = arith.constant 10 : i32
+  %c2_i32 = arith.constant 1 : i32
+  %c0_i32_0 = arith.constant 0 : i32
+  
+  // CHECK: scf.for %arg0
+  // CHECK: scf.for %arg1
+  // CHECK: scf.for %arg2
+
+  scf.for %arg0 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+    scf.for %arg1 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+      scf.for %arg2 = %c0_i32 to %c1_i32 step %c2_i32  : i32 {
+        
+        // Read effect on "A" dominates write.
+        // Causes conflict "A" --> not movable.
+        // CHECK: "test.test_effects_read_A"() : () -> ()
+        // CHECK: "test.test_effects_write_A"() : () -> ()
+
+        "test.test_effects_read_A"() : () -> ()
+        "test.test_effects_write_A"() : () -> ()
----------------
mbagherbeikTT wrote:

If it's safe to assume that, because test_effects_read_A() doesn't take any input and doesn't have a result, the read data "isn't used," It would be idempotent. 

If that can't be assumed, then the conservative option is to flag it as a conflict on the resource.

If the assumption CAN be made, I can check if the read data "is used" when mapping conflicts.  



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