[PATCH] D78996: [MLIR][LINALG] Convert Linalg to Linalg

[Ehsan via Phabricator via llvm-commits]

dfki-ehna marked 13 inline comments as done.
dfki-ehna added inline comments.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/TensorsToBuffers.cpp:122
+    // Mark return operations illegal as long as they return values.
+    target.addDynamicallyLegalOp<mlir::ReturnOp>(
+        [](mlir::ReturnOp returnOp) { return returnOp.getNumOperands() == 0; });
----------------
ftynse wrote:
> This looks like it would make impossible to have simple helper functions that have no Linalg in them.
> 
> ```
> func @square(%arg0: f32) {
>   %0 = mulf %arg0, %arg0 : f32
>   return %0 : f32
> }
> 
> func @foo(...) {
>   linalg.generic (...buffers...) {
>     %0 = call @square(...) : (f32) -> f32
>     linalg.yield %0 : f32
>   }
> }
> ```
Thanks for mentioning this. We addressed this issue with a quick fix to cover more test cases in this PR. However, the combination of FunctionAndBlockSignatureConverter and NonVoidToVoidReturnOpConverter of BufferAssignment would still fail in cases like the following:
```
func @function(%arg0: f32, %arg1: tensor<4xf32>) -> (f32, f32) {
  %0 = mulf %arg0, %arg0 : f32
  return %0, %0 : f32, f32
}
```
FunctionAndBlockSignatureConverter converts the function to:
```
func @function(%arg0: f32, %arg1: memref<4xf32>, f32, f32) {
  %0 = mulf %arg0, %arg0 : f32
  return %0, %0 : f32, f32
}
```
but the return operation remains unchanged and leaves the IR in a broken state.
These converters must be fixed more fundamentally and we are going to do so in the next PR.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78996/new/

https://reviews.llvm.org/D78996