[PATCH] D78498: [mlir] [linalg] Only promote selected buffers.

Mon Apr 20 10:48:29 PDT 2020

poechsel marked 5 inline comments as done.
poechsel added inline comments.

================
Comment at: mlir/test/lib/DeclarativeTransforms/TestLinalgTransformPatterns.td:153
+def : Pat<(MatmulOp:$op $_, $_, $_),
+          (PromoteSelectedSubviewsLinalgOp<[0, 1], "first_view_promotion">),
+          [(Constraint<And<[
----------------
ftynse wrote:
> I don't follow this. The test name suggests you promote the _first_ view, but the list contains operands with indices 0 and 1... The .mlir file also seems to check only for one copy. Consider using only `[0]` here and `CHECK-NOT` for extra copies before matmul.
Nice catch! 

I believe there is a problem with `test/Dialect/Linalg/transform-patterns.mlir` and that the test is passing even when it should fail:
- in my case I should have had a fallback on line 439 where I'm checking the ssa address of arg1 with the one it should have if no promotion happens.
- since D77676 promotion shouldn't generate `linalg.slice` operations but this test is still checking for their presence (ex: line 393).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78498