Page MenuHomePhabricator

[mlir] Generare .cpp.inc files for dialects.
ClosedPublic

Authored by stellaraccident on Jun 28 2021, 4:58 PM.

Details

Summary
  • Previously, we were only generating .h.inc files. We foresee the need to also generate implementations and this is a step towards that.
  • Discussed in https://llvm.discourse.group/t/generating-cpp-inc-files-for-dialects/3732/2
  • Deviates from the discussion above by generating a default destructor in the .cpp.inc file (and adding a tablegen bit that disables this in case if this is user provided). The Test dialect uses this feature but others appear not to.
  • Generating the destructor started as a way to flush out the missing includes (produces a link error), but it is a strict improvement on its own that is worth doing (i.e. by emitting key methods in the .cpp file, we root vtables in one translation unit, which is a non-controversial improvement).

Diff Detail

Event Timeline

stellaraccident requested review of this revision.Jun 28 2021, 4:58 PM

Generally looks good to me.

mlir/include/mlir/IR/OpBase.td
272

I couldn't tell from the description, is this supposed to be temporary?

mlir/tools/mlir-tblgen/DialectGen.cpp
63

nit: Drop else after returns.

269

Drop trivial braces here.

lattner accepted this revision.Jun 28 2021, 5:39 PM

This looks great to me Stella, nice work. I don't have any additional comments on top of River's

This revision is now accepted and ready to land.Jun 28 2021, 5:39 PM
mlir/include/mlir/IR/OpBase.td
272

No: I added it because the test dialect does define its own destructor and I needed to support that (it appears to be the only one). Open to other ways to do this, but it seemed neighborly to leave the option open to folks who need it.

stellaraccident edited the summary of this revision. (Show Details)Jun 28 2021, 5:49 PM

Patch all dialects

Update bazel build files.

stellaraccident edited the summary of this revision. (Show Details)Jun 28 2021, 8:08 PM
rriddle accepted this revision.Jun 29 2021, 10:54 AM
stellaraccident edited the summary of this revision. (Show Details)

Rename hasExplicitDestructor to hasNonDefaultDestructor (on a second read, this seemed like an improvement to me).

Rebase and fix standalone dialect template.

This revision was landed with ongoing or failed builds.Jun 29 2021, 1:12 PM
This revision was automatically updated to reflect the committed changes.
newling added inline comments.
mlir/examples/standalone/lib/Standalone/StandaloneDialect.cpp
15

This seems to have broken the standalone example for me. Is the standalone example tested?

mehdi_amini added inline comments.Sep 23 2021, 10:46 AM
mlir/examples/standalone/lib/Standalone/StandaloneDialect.cpp
15

It should be: is it broken at HEAD?