This is an archive of the discontinued LLVM Phabricator instance.

[mlir][index] Add boilerplate for the `index` dialect
ClosedPublic

Authored by Mogball on Oct 11 2022, 9:14 AM.

Details

Summary

This patch introduces the index dialect and associated boilerplate for
adding ops and enums (comparison predicates).

Diff Detail

Event Timeline

Mogball created this revision.Oct 11 2022, 9:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2022, 9:14 AM
Mogball requested review of this revision.Oct 11 2022, 9:14 AM
Mogball updated this revision to Diff 466837.Oct 11 2022, 9:30 AM

update commandline test

rriddle accepted this revision.Oct 12 2022, 12:18 AM

This skeleton LGTM as an initial starting point.

mlir/include/mlir/Dialect/Index/IR/IndexDialect.td
84–86

Can you drop these until needed?

mlir/lib/Dialect/Index/IR/IndexDialect.cpp
13
mlir/lib/Dialect/Index/IR/IndexOps.cpp
14

Same here.

This revision is now accepted and ready to land.Oct 12 2022, 12:18 AM
jpienaar accepted this revision.Oct 12 2022, 6:02 AM
jpienaar added inline comments.
mlir/include/mlir/Dialect/Index/IR/IndexDialect.td
25

index type (builtin.index) ? Just to be clear that for here it is the concrete type instance of interest

38

the maximum -> maximum

41

You've switched here to consider what happens for 32 bit targets, but it's a bit abrupt/you need to read to the end before having the context for the start.

mlir/lib/Dialect/Index/IR/IndexAttrs.cpp
2

Same

mlir/lib/Dialect/Index/IR/IndexDialect.cpp
2

No need to specify file type in .cpp file

nicolasvasilache accepted this revision.Oct 12 2022, 6:08 AM

Thanks for pushing on this @Mogball !

Mogball updated this revision to Diff 467333.Oct 12 2022, 6:06 PM
Mogball marked 7 inline comments as done.

Address reviewer comments

Mogball marked an inline comment as done.Oct 12 2022, 6:06 PM
Mogball added inline comments.
mlir/include/mlir/Dialect/Index/IR/IndexDialect.td
41

I clarified the paragraph a bit.

Mogball updated this revision to Diff 467334.Oct 12 2022, 6:22 PM

sink changes

This revision was automatically updated to reflect the committed changes.
fmayer added a subscriber: fmayer.EditedOct 21 2022, 11:35 AM

This has broken our sanitizer buildbot: https://lab.llvm.org/buildbot/#/builders/5/builds/28427/steps/9/logs/stdio

[4543/6866] Building IndexOpsDialect.h.inc...
FAILED: tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsDialect.h.inc /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsDialect.h.inc 
cd /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan && /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/mlir-tblgen -gen-dialect-decls -dialect=index -I /b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/include -I/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include -I/b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/mlir/include /b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR/IndexOps.td --write-if-changed -o tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsDialect.h.inc -d tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsDialect.h.inc.d
/b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR/IndexOps.td:25:35: error: Variable not defined: 'NoSideEffect'
    : Op<IndexDialect, mnemonic, [NoSideEffect] # traits>;
                                  ^
[4544/6866] Building IndexOpsTypes.cpp.inc...
FAILED: tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsTypes.cpp.inc /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsTypes.cpp.inc 
cd /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan && /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/mlir-tblgen -gen-typedef-defs -typedefs-dialect=index -I /b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/include -I/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include -I/b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/mlir/include /b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR/IndexOps.td --write-if-changed -o tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsTypes.cpp.inc -d tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsTypes.cpp.inc.d
/b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR/IndexOps.td:25:35: error: Variable not defined: 'NoSideEffect'
    : Op<IndexDialect, mnemonic, [NoSideEffect] # traits>;
                                  ^
[4545/6866] Building BufferizationOps.cpp.inc...
[4546/6866] Building BufferizationOps.h.inc...
[4547/6866] Building IndexOpsTypes.h.inc...
FAILED: tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsTypes.h.inc /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsTypes.h.inc 
cd /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan && /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/mlir-tblgen -gen-typedef-decls -typedefs-dialect=index -I /b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/include -I/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include -I/b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/mlir/include /b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR/IndexOps.td --write-if-changed -o tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsTypes.h.inc -d tools/mlir/include/mlir/Dialect/Index/IR/IndexOpsTypes.h.inc.d
/b/sanitizer-x86_64-linux-fast/build/llvm-project/mlir/include/mlir/Dialect/Index/IR/IndexOps.td:25:35: error: Variable not defined: 'NoSideEffect'
    : Op<IndexDialect, mnemonic, [NoSideEffect] # traits>;

Thanks. The fix should've been pushed out already