This is an archive of the discontinued LLVM Phabricator instance.

[mlir][index] Add `index` dialect ops and attributes
ClosedPublic

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

Details

Summary

This patch adds the definitions for the operations and attributes (just
one enum attribute) for the index dialect.

Depends on D135688

Diff Detail

Event Timeline

Mogball created this revision.Oct 11 2022, 9:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2022, 9:33 AM
Mogball requested review of this revision.Oct 11 2022, 9:33 AM
rriddle accepted this revision.Oct 12 2022, 12:24 AM
rriddle added inline comments.
mlir/include/mlir/Dialect/Index/IR/CMakeLists.txt
4–5 ↗(On Diff #466838)

Can you fix this in the base?

mlir/include/mlir/Dialect/Index/IR/IndexAttrs.h
12 ↗(On Diff #466838)

Same here, can you sink this into the base?

mlir/include/mlir/Dialect/Index/IR/IndexOps.td
271

Can you use a non-negative number here?

This revision is now accepted and ready to land.Oct 12 2022, 12:24 AM
jpienaar accepted this revision.Oct 12 2022, 6:19 AM
jpienaar added inline comments.
mlir/include/mlir/Dialect/Index/IR/IndexEnums.td
22

Micro nit: add additional space here and next so we have aligned columns

mlir/include/mlir/Dialect/Index/IR/IndexOps.td
33

If you add type inference include file you'd get most builders with inferred return types automatically given this is trivial to construct return type.

351

Is the goal of adding integer value to doc in case of generic dump?

I'd much rather when we generate the enum attr doc that we have those values and then here just link to them. Avoids these being able to get out of sync.

365

Newline before `mlir to ensure markdown formatter handles it correctly

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

review comments

Mogball added inline comments.Oct 14 2022, 8:41 AM
mlir/include/mlir/Dialect/Index/IR/IndexOps.td
33

Ah! so that's how you get them! Would have never guessed

351

copy pasta

This revision was automatically updated to reflect the committed changes.

Note NoSideEffect has changed.