This is an archive of the discontinued LLVM Phabricator instance.

Split `ElementwiseMappable` trait into four more precise traits.
ClosedPublic

Authored by frgossen on Mar 1 2021, 3:43 AM.

Details

Summary

Some elementwise operations are not scalarizable, vectorizable, or tensorizable.
Split ElementwiseMappable trait into the following, more precise traits.

  • Elementwise
  • Scalarizable
  • Vectorizable
  • Tensorizable

This allows for reuse of Elementwise in dialects like HLO.

Diff Detail

Event Timeline

frgossen created this revision.Mar 1 2021, 3:43 AM
frgossen requested review of this revision.Mar 1 2021, 3:43 AM
herhut added a subscriber: herhut.Mar 1 2021, 8:42 AM
herhut added inline comments.
mlir/include/mlir/IR/OpBase.td
1794

Would it make sense to define something like ElementwiseMappable.traits as a shortcut to all of these? As is done in https://reviews.llvm.org/D97636?

frgossen updated this revision to Diff 327393.Mar 2 2021, 2:03 AM

Address comments

frgossen marked an inline comment as done.Mar 2 2021, 2:03 AM
frgossen added inline comments.
mlir/include/mlir/IR/OpBase.td
1794

Thanks!

frgossen marked an inline comment as done.Mar 2 2021, 2:04 AM
frgossen added a reviewer: herhut.
herhut accepted this revision.Mar 2 2021, 4:51 AM

Just nits for comments. Looks good, thanks!

mlir/include/mlir/IR/OpBase.td
1792

nit: can be applied to ...

Also, replicate the comment for both entries rather than reuse it here to make it obvious to which def it applies.

1797

nit: Tensorizable?

mlir/include/mlir/IR/OpDefinition.h
1307

Can you copy the comment to the other def and split it? So that automated tooling can provide a description for the def?

This revision is now accepted and ready to land.Mar 2 2021, 4:51 AM
frgossen updated this revision to Diff 327433.Mar 2 2021, 6:29 AM
frgossen marked 3 inline comments as done.

Address comments

This revision was landed with ongoing or failed builds.Mar 2 2021, 6:31 AM
This revision was automatically updated to reflect the committed changes.
silvas added inline comments.Mar 2 2021, 11:59 AM
mlir/include/mlir/IR/OpDefinition.h
1216

don't mention scalars. this trait simply constrains the ways that ops can use vectors or tensors, if they have one as an operand.

1259

s/per/of the/

1263

"based on the same op's behavior on scalars".

1288

perhaps mention that this is just Tensorizable with s/tensor/vector. (that way folks can read the larger documentation on Tensorizable to understand Vectorizable)

1291

s/arguments/elements/

1317

s/select_scalar_pred above/scalar_pred below/

1326

stray ).