This is an archive of the discontinued LLVM Phabricator instance.

Restructure the Test dialect ODS to include the AttrDef in TestOps.td (NFC)
ClosedPublic

Authored by mehdi_amini on Nov 5 2021, 6:19 PM.

Details

Summary

This structure is necessary to be able to use AttrDef as arguments on operations.

Diff Detail

Event Timeline

mehdi_amini created this revision.Nov 5 2021, 6:19 PM
mehdi_amini requested review of this revision.Nov 5 2021, 6:19 PM
rriddle accepted this revision.Nov 5 2021, 10:59 PM

It would likely be cleaner in the long run to move the Test dialect definition to its own .td file.

This revision is now accepted and ready to land.Nov 5 2021, 10:59 PM

It would likely be cleaner in the long run to move the Test dialect definition to its own .td file.

Right, but we would still need to include the "TestAttrDefs.td" in the "TestOps.td" file right?

It would likely be cleaner in the long run to move the Test dialect definition to its own .td file.

Right, but we would still need to include the "TestAttrDefs.td" in the "TestOps.td" file right?

  • To get the parseAttr/printAttr accessors to be auto-generated?

Not technically, the constraint there (which should really be cleaned up now that attributes/types can be defined in ODS) is that there is an instance of DialectAttr/DialectType that uses that dialect.

  • To access the attributes in the op arguments field?

Yeah, the file would need to be included to access the ODS definitions for the attributes.

mlir/test/lib/Dialect/Test/TestOps.td
57–58

Why is this included here and not at the top of the file with the other includes?

Yeah, the file would need to be included to access the ODS definitions for the attributes.

Right, that's what I meant.

mlir/test/lib/Dialect/Test/TestOps.td
57–58

The AttrDef needs to access the Test_Dialect def. We could move the Test_Dialect def outside of the ops file and include it in both...