This is an archive of the discontinued LLVM Phabricator instance.

[mlir][IR] Remove the concept of `OperationProperties`
ClosedPublic

Authored by rriddle on Feb 4 2021, 4:06 PM.

Details

Summary

These properties were useful for a few things before traits had a better integration story, but don't really carry their weight well these days. Most of these properties are already checked via traits in most of the code. It is better to align the system around traits, and improve the performance/cost of traits in general.

Diff Detail

Event Timeline

rriddle created this revision.Feb 4 2021, 4:06 PM
rriddle requested review of this revision.Feb 4 2021, 4:06 PM
jpienaar added inline comments.Feb 4 2021, 4:23 PM
mlir/lib/Dialect/Affine/IR/AffineOps.cpp
391

Alternatively a helper function may make this nicer to read

!isKnownIsolatedFromAbove(region->getParentOp())

and it just simply queries the trait.

silvas accepted this revision.Feb 4 2021, 5:25 PM

Nice. Looks good. I assume you checked this doesn't regress anything significantly in practice?

mlir/include/mlir/IR/Operation.h
485

nit: absOp makes me think of mathematical abs. spell out abstractOp

This revision is now accepted and ready to land.Feb 4 2021, 5:25 PM
rriddle marked 2 inline comments as done.Feb 8 2021, 12:43 PM

Nice. Looks good. I assume you checked this doesn't regress anything significantly in practice?

Yeah, for the benchmarks I was running there was no visible change in execution outside of noise.

mlir/lib/Dialect/Affine/IR/AffineOps.cpp
391

I'd prefer to keep this consistent with other traits for now.

rriddle updated this revision to Diff 322196.Feb 8 2021, 12:50 PM
rriddle marked an inline comment as done.

comments

This revision was automatically updated to reflect the committed changes.