This is an archive of the discontinued LLVM Phabricator instance.

[mlir][IR] Define the singleton builtin types in ODS instead of C++
ClosedPublic

Authored by rriddle on Dec 14 2020, 8:48 PM.

Details

Summary

This exposes several issues with the current generation that this revision also fixes.

  • TypeDef now allows specifying the base class to use when generating.
  • TypeDef now inherits from DialectType, which allows for using it as a TypeConstraint
  • Parser/Printers are now no longer generated in the header(removing duplicate symbols), and are now only generated when necessary.
    • Now that generatedTypeParser/Printer are only generated in the definition file, existing users will need to manually expose this functionality when necessary.
  • ::get() is no longer generated for singleton types, because it isn't necessary.

Diff Detail

Event Timeline

rriddle created this revision.Dec 14 2020, 8:48 PM
rriddle requested review of this revision.Dec 14 2020, 8:48 PM
jdd added inline comments.Dec 15 2020, 12:24 AM
mlir/include/mlir/IR/OpBase.td
2422

I think there should be a :: before the dialect.cppNamespace.

2423

What if someone overrides the cppClassName field? I was considering doing this exact thing a while back.

mlir/lib/TableGen/Constraint.cpp
63

I'm pretty sure this'll break the documentation generation.

jdd added inline comments.Dec 15 2020, 9:13 AM
mlir/lib/TableGen/Constraint.cpp
63

No it won't. Sorry -- didn't see which file it was in.

mlir/tools/mlir-tblgen/TypeDefGen.cpp
537

I think this'll break users who have a separate file to handle their type code. They'll just have to move the callsite, but something to note.

lattner accepted this revision.Dec 15 2020, 10:39 AM

This is really awesome River, I love adopting and dogfooding ODS features in the builtin/std dialect. Thanks for also fixing the TypeConstraint issue.

This revision is now accepted and ready to land.Dec 15 2020, 10:39 AM
rriddle updated this revision to Diff 311996.Dec 15 2020, 12:26 PM
rriddle marked 4 inline comments as done.

Update

rriddle updated this revision to Diff 311997.Dec 15 2020, 12:35 PM

Reformat docs

rriddle edited the summary of this revision. (Show Details)Dec 15 2020, 12:36 PM
rriddle marked an inline comment as done.
rriddle added inline comments.
mlir/include/mlir/IR/OpBase.td
2422

Most of the usages of cppNamespace in mlir have already been prefixing the :::

https://github.com/llvm/llvm-project/search?p=2&q=cppNamespace
(also github search is horrible)

2423

Thanks for the catch, fixed.

mlir/tools/mlir-tblgen/TypeDefGen.cpp
537

Added an explicit bullet in the description, is there any thing else you'd like to see for this?

jdd accepted this revision.Dec 15 2020, 12:41 PM

This looks great!

jdd added inline comments.Dec 15 2020, 12:42 PM
mlir/tools/mlir-tblgen/TypeDefGen.cpp
537

Nope

This revision was landed with ongoing or failed builds.Dec 15 2020, 1:51 PM
This revision was automatically updated to reflect the committed changes.
rriddle marked an inline comment as done.

This commit breaks my build, am I doing something wrong?

mlir/lib/IR/BuiltinTypes.cpp
28
$ mkdir build

$ cd build

$ cmake \
-G Ninja \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_C_COMPILER=$(command -v clang) \
-DCMAKE_CXX_COMPILER=$(command -v clang++) \
-DLLVM_CCACHE_BUILD=ON \
-DLLVM_ENABLE_PROJECTS=all \
-DLLVM_ENABLE_WARNINGS=OFF \
../llvm
...

$ ninja
...
[7082/8536] Building CXX object projects/debuginfo-tests/CMakeFiles/check-gdb-mlir-support.dir/llvm-prettyprinters/gdb/mlir-support.cpp.o
FAILED: projects/debuginfo-tests/CMakeFiles/check-gdb-mlir-support.dir/llvm-prettyprinters/gdb/mlir-support.cpp.o
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /home/nathan/usr/bin/ccache /home/nathan/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/debuginfo-tests -I/home/nathan/src/llvm-project/debuginfo-test$In file included from /home/nathan/src/llvm-project/debuginfo-tests/llvm-prettyprinters/gdb/mlir-support.cpp:2:
/home/nathan/src/llvm-project/llvm/../mlir/include/mlir/IR/BuiltinTypes.h:639:10: fatal error: 'mlir/IR/BuiltinTypes.h.inc' file not found
#include "mlir/IR/BuiltinTypes.h.inc"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
...
$ git bisect log
# bad: [f314bcffa3c6083079831dde4e7bc124a25e0c04] [llvm-reduce][test] Make remove-alias.ll CHECK patterns more specific after D90302
# good: [5a1bc69f811059b8f62d381e3526d92fffa7d91a] [clangd] NFC: Add client-side logging for remote index requests
git bisect start 'origin/main~1023' 'origin/main~2047'
# good: [2a2268a6db17cbbef54c1b80f74a04849831c997] [VE][NFC] Sort VEISD operations
git bisect good 2a2268a6db17cbbef54c1b80f74a04849831c997
# bad: [97c006aabb6c831d68204bcb4aad8670af695618] [AArch64] Add a GPR64x8 register class
git bisect bad 97c006aabb6c831d68204bcb4aad8670af695618
# bad: [c10757200d89e59a52ab8cbbc68178eeafaa3600] Revert "Ensure SplitEdge to return the new block between the two given blocks"
git bisect bad c10757200d89e59a52ab8cbbc68178eeafaa3600
# bad: [8c4e55762d8b7a07546a5db18e33ccc6a9d97002] [docs][unittest][Go][StackProtector] Migrate deprecated DebugInfo::get to DILocation::get
git bisect bad 8c4e55762d8b7a07546a5db18e33ccc6a9d97002
# good: [cfa1010c42429f689186125270dfbad7d6a1c01d] [clangd] Provide suggestions with invalid config keys
git bisect good cfa1010c42429f689186125270dfbad7d6a1c01d
# good: [31845199094418173a3beadb786767b860bfda03] [lld-macho] Don't emit rebase opcodes for relocs in TLV sections
git bisect good 31845199094418173a3beadb786767b860bfda03
# good: [a7deedc414e2abbe4b9557d46e896a5bdba25f2b] [NFC][Tests][SimplifyCFG] Trim whitespaces at the end of lines
git bisect good a7deedc414e2abbe4b9557d46e896a5bdba25f2b
# bad: [59decf8e9c3d86472073266ad8ee1cc699d94525] [clang] Migrate deprecated DebugInfo::get to DILocation::get
git bisect bad 59decf8e9c3d86472073266ad8ee1cc699d94525
# bad: [95019de8a122619fc038c9fe3c80e625e3456bbf] [mlir][IR] Define the singleton builtin types in ODS instead of C++
git bisect bad 95019de8a122619fc038c9fe3c80e625e3456bbf
# good: [e1133179587dd895962a2fe4d6eb0cb1e63b5ee2] [NFCI][SimplifyCFG] Add basic scaffolding for gradually making the pass DomTree-aware
git bisect good e1133179587dd895962a2fe4d6eb0cb1e63b5ee2
# first bad commit: [95019de8a122619fc038c9fe3c80e625e3456bbf] [mlir][IR] Define the singleton builtin types in ODS instead of C++

This commit breaks my build, am I doing something wrong?

I believe it is a missing dependency in CMake, so it is non-deterministic and your bisection will not yield the right culprit.
See https://reviews.llvm.org/D87159

To bisect a build failure, is is more reliable to just build the target that is failing, i.e.: ninja projects/debuginfo-tests/CMakeFiles/check-gdb-mlir-support.dir/llvm-prettyprinters/gdb/mlir-support.cpp.o