[Mlir-commits] [mlir] [MLIR][RemoveDeadValues] Mark arguments of a public function Live (PR #162038)

[xin liu]

================
@@ -569,6 +569,24 @@ module @return_void_with_unused_argument {
     call @fn_return_void_with_unused_argument(%arg0, %unused) : (i32, memref<4xi32>) -> ()
     return %unused : memref<4xi32>
   }
+
+  // the function signature is immutable because it is public.
+  func.func public @immutable_fn_with_unused_argument(%arg0: i32, %arg1: memref<4xf32>) -> () {
+    return
+  }
+
+  // CHECK-LABEL: func.func @main2
+  // CHECK: %[[MEM:.*]] = memref.alloc() : memref<4xf32>
+  // CHECK: %[[UNUSED:.*]] = arith.constant 0 : i32
+  // CHECK: call @immutable_fn_with_unused_argument(%[[UNUSED]], %[[MEM]]) : (i32, memref<4xf32>) -> ()
+  func.func @main2() -> () {
+    %one = arith.constant 1 : i32
+    %scalar = arith.addi %one, %one: i32
+    %mem = memref.alloc() : memref<4xf32>
+
+    call @immutable_fn_with_unused_argument(%scalar, %mem) : (i32, memref<4xf32>) -> ()
+    return
+  }
 }
----------------
navyxliu wrote:

hi, @joker-eph 
I did add a testcase as you said. Then I realize that it's non-trivial to propagate liveness in RemoveDeadValues. 

Here is a testcase only for RegionBranchOpinterface. From %0 is live at line 11, we need to mark %0 is live at line 2. After that, we need to mark %c1_i32 at line 4 and c0_i32 at line 7 live as well.  In order words, we need to walk function @main3 *preorder* + backward. 

```
     1	  func.func @main3(%arg0: i1) {
     2	    %0 = scf.if %arg0 -> (i32) {
     3	      %c1_i32 = arith.constant 1 : i32
     4	      scf.yield %c1_i32 : i32
     5	    } else {
     6	      %c0_i32 = arith.constant 0 : i32
     7	      scf.yield %c0_i32 : i32
     8	    }
     9	    %mem = memref.alloc() : memref<4xf32>
    10	
    11	    call @immutable_fn_with_unused_argument(%0, %mem) : (i32, memref<4xf32>) -> ()
    12	    return
    13	  }
```

I manage to fix this in [propagateBackward](https://github.com/llvm/llvm-project/pull/162038/files#diff-29f228261d7d479abc606824f895cae235ca99cc8676da9d66a2fd0f6e672fe8R407). It pretty much redo what liveness analysis has done. TBH, I don't think this is the right way to proceed. RemoveDeadValues should keep its own single responsibility. 

I take a step back and think about why we end up here. The very reason we try to propagate liveness in it because:
1) liveness is immutable. 
2) We somehow need to update the NonLive arguments of a public function. 

How about we just introduce a new pass: 'privatize-public-function' right before 'remove-dead-values'. 

1. It deploys separation of interface and implementation.
2. If nothing changes, we preserve liveness. Otherwise, we invalidate it and let remove-dead-value recompute.  

Here is a demo what this pass transforms. 
i think we can waive cost model because we don't clone function body. We just create a
thin wrapper. 

```
public void foo(int unused){...}

void main() {
arg = compute();
call foo(arg);
}
=> 
public void foo(int unused) { // interface
return __foo_impl(unused);
}

private void __foo_impl(int unused) { //implementation, new function.
... // the function body of the original foo.
}

void main() {
arg = compute();
call __foo_impl(arg);
}
```

This is my prototype here. do you think it's more feasible solution? 
https://github.com/navyxliu/llvm-project/commit/b73f5371715e1968810aac46eb47a166c63064df#diff-904855c22d662d8afbc11c40fe2906259836ba53e907e1cc899e6355358ec482



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