[mlir] [MLIR][Linalg] Add aggregate ops decomposition pass and softmax decom… (PR #97582)
Andrzej Warzyński
llvmlistbot at llvm.org
Tue Jul 9 01:45:56 PDT 2024
================
@@ -211,31 +213,61 @@ func.func @softmax(%arg0: tensor<2x16x32xf32>, %dst: tensor<2x16x32xf32>) -> ten
// CHECK-SAME: %[[ARG0:[a-zA-Z0-9_]+]]: tensor<2x16x32xf32>, %[[DST:[a-zA-Z0-9_]+]]: tensor<2x16x32xf32>) -> tensor<2x16x32xf32> {
// CHECK-DAG: %[[D1:.+]] = tensor.empty() : tensor<2x16xf32>
// CHECK-DAG: %[[CST:.+]] = arith.constant -3.40282347E+38 : f32
-// CHECK: %[[D2:.+]] = linalg.fill ins(%[[CST]] : f32) outs(%[[D1]] : tensor<2x16xf32>) -> tensor<2x16xf32>
+// CHECK: %[[D2:.+]] = linalg.generic {indexing_maps = [#[[$MAP2]], #[[$MAP3]]],
----------------
banach-space wrote:
> I preserved the original naming for the diff to be readable.
We should avoid optimising our patches to work around the limitations of GitHub. If GitHub rendering is hard to parse, I will happily just clone a PR and review locally. Lets optimise for maintainability instead.
> I also don't find the renaming very helpful or valuable.
I disagree - IMO it's both helpful and valuable. With `linalg.fill`, it doesn't matter _that much_ whether it's `%[[D2:.+]] ` or `%[[FILL_1:.+]]` - it's clear what the Op is. But your patch replaces `linalg.fill` with `linalg.generic`, which makes reading this rather dense test even trickier. To me, more descriptive LIT variables _are_ helpful - they make parsing tests much easier.
https://github.com/llvm/llvm-project/pull/97582
More information about the Mlir-commits
mailing list