[Mlir-commits] [mlir] [mlir][memref] Fix hoist-static-allocs option of buffer-results-to-out-params when function parameters are returned (PR #102093)

[llvmlistbot at llvm.org]

================
@@ -120,10 +128,15 @@ static LogicalResult updateReturnOps(func::FuncOp func,
     }
     OpBuilder builder(op);
     for (auto [orig, arg] : llvm::zip(copyIntoOutParams, appendedEntryArgs)) {
-      if (hoistStaticAllocs && isa<memref::AllocOp>(orig.getDefiningOp()) &&
-          mlir::cast<MemRefType>(orig.getType()).hasStaticShape()) {
+      bool mayHoistStaticAlloc =
+          hoistStaticAllocs &&
+          mlir::cast<MemRefType>(orig.getType()).hasStaticShape();
+      if (mayHoistStaticAlloc &&
+          isa_and_nonnull<memref::AllocOp>(orig.getDefiningOp())) {
         orig.replaceAllUsesWith(arg);
         orig.getDefiningOp()->erase();
+      } else if (mayHoistStaticAlloc && isFunctionArgument(orig)) {
----------------
Menooker wrote:

Actually I am not sure if it is ok to avoid the memcpy for this case...

Consider the simple function:

```mlir
func add1(%0: memref<...>) -> memref<...> {
    my.add(%0)
    return %0
}
```

This pass will change the signature to:


```mlir
func add1(%0: memref<...>, %out: memref<...>)  {
 ....
}
```

If the user of this pass is unaware of the in-place-return behavior, they may pass different buffers for `%0` and %out`. The safest way is to copy %0 to %out. It will always meet the expectation of the users.

If we remove the memcpy from %0 to %out, actually %out is an unused parameter, and we should expect the user aware of this in-place-return behavior. If we turn it on by default, will it be a surprise to them?

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