[PATCH] D77067: [mlir][Linalg] Create a tool to generate named Linalg ops from a Tensor Comprehensions-like specification.

[Alex Zinenko via Phabricator via llvm-commits]

ftynse added inline comments.


================
Comment at: mlir/test/mlir-linalg-ods-gen/test-linalg-ods-gen.tc:5
+
+// RUN: mlir-linalg-ods-gen %s -gen-ods-decl=1 -o %t \
+// RUN: && cat %S/../../include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td %t \
----------------
nicolasvasilache wrote:
> silvas wrote:
> > layering-wise would prefer to not test this here. If needed, we can add a separate test elsewhere that does this .td -> .inc file check. Strictly speaking what ends up in the .inc file is not really the concern of this component, only the contents of the .td file.
> > 
> This was suggested by @ftynse to show the ODS is valid and how it connects to tblgen by mirroring this test: https://github.com/llvm/llvm-project/blob/master/mlir/test/mlir-tblgen/llvm-intrinsics.td#L11.
> 
> I am fine either way, would just like consensus on this before reverting back to the previous state.
> Please reopen if you feel strongly about this.
> @ftynse any strong opinion?
I'm not sure I understand what is the concern here? The `ODS` check verifies the content of the produced .td file, _not_ the result of feeding that .td file to `mlir-tblgen -gen-op-defs`, which is indeed a separate concern. The `IMPL` check verifies the implementations of methods that are declared in the `.td` file and there is simply no other place where we can verify them.

The staging here is:
1a. mlir-linalg-ods-gen -gen-ods-decl %this_file% > ods.td
1b. mlir-linalg-ods-gen -gen-impl %this_file% > impl.cc
2a. mlir-tblgen -gen-op-decl ods.td > ods.h
2b. mlir-tblgen -gen-op-decl ods.td > ods.cc
3. include impl.cc and ods.cc into the implementation file; and ods.h into the header file.

@nicolasvasilache the test you referenced also has `RUN` lines making sure `mlir-tblgen` can consume what the first stage produces. Consider adding them here as well. This could help detect cases of ODS syntax change (the simple syntactic test passes, but not the piping check). That's why there is only a trivial check to make sure FileCheck eats something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77067