This is an archive of the discontinued LLVM Phabricator instance.

[mlir][linalg][python] Add auto-generated file warning (NFC).
ClosedPublic

Authored by gysit on Jul 12 2021, 5:08 AM.

Details

Summary

Annotate LinalgNamedStructuredOps.yaml with a comment stating the file is auto-generated and should not be edited manually.

Diff Detail

Event Timeline

gysit created this revision.Jul 12 2021, 5:08 AM
gysit requested review of this revision.Jul 12 2021, 5:08 AM
nicolasvasilache accepted this revision.Jul 12 2021, 5:20 AM
This revision is now accepted and ready to land.Jul 12 2021, 5:20 AM

If LinalgNamedStructuredOps.yaml is generated by a script, would it be possible to remove it from the repository entirely and regenerate it during the build process? I feel that checking out a generated file into the repository may lead to some inconsistencies between the script and the generated version upstream if someone forgets to commit an updated version of LinalgNamedStructuredOps.yaml after each update to dump_oplib.py.

gysit added a comment.Jul 12 2021, 5:49 AM

If LinalgNamedStructuredOps.yaml is generated by a script, would it be possible to remove it from the repository entirely and regenerate it during the build process? I feel that checking out a generated file into the repository may lead to some inconsistencies between the script and the generated version upstream if someone forgets to commit an updated version of LinalgNamedStructuredOps.yaml after each update to dump_oplib.py.

It is a known issue but a non-trivial one. Not everyone has Python enabled and specifying dependencies between custom cmake targets is notoriously buggy (see for example, https://reviews.llvm.org/D105272 and the associated discussion https://llvm.discourse.group/t/missing-build-cmake-tblgen-dependency/3727/10). Additionally, running dump_oplib.py would require a two stage build process since it depends on mlir. At least for now, committing LinalgNamedStructuredOps.yaml seems like the better choice.

Right, seems reasonable enough until we figure out something better. Thanks for the clarification and the pointers!