This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Linalg] Add a softmax op
ClosedPublic

Authored by qcolombet on Jun 21 2023, 6:32 AM.

Details

Summary

This patch adds a softmax op.
For now, nothing interesting happens, we can only do a round trip.
Later patches will add the tiling interface and the lowering of this op to a sequence of simpler ops.

This is graduating the linag_ext.softmax op from iree to LLVM.

Original implementation from @harsh.
@nicolasvasilache co-authored this patch.

Diff Detail

Event Timeline

qcolombet created this revision.Jun 21 2023, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2023, 6:32 AM
qcolombet requested review of this revision.Jun 21 2023, 6:32 AM
rengolin added inline comments.Jun 21 2023, 6:54 AM
mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
100

Would it make sense to allow more than one dimension here?

If your tensor is MB x Head x TileX x TileY you want the max over TileX x TileY, no?

Thanks! Some minor comments.

mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
125

Any specific reason for having this wrapper? Can we use the method generated by tablegen directly getInput? Same for getOutput.

144

I cannot fully understand the sentence, maybe something like: Implement functions necessary for DestinationStyleOpInterface.

mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
2148

I don't think getting the pointer is needed here. Below we can simply write:

if (failed(...)
  emitOpError("...")
2166

Can you please comment on why we need this folder?

mlir/test/Dialect/Linalg/invalid.mlir
743

nit optional: I would align ins and outs.

nicolasvasilache accepted this revision.Jun 29 2023, 3:16 AM
nicolasvasilache added inline comments.
mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
100

let's extend on a per-need basis if you don't mind, proabably it makes sense yes

125

yes let's drop this

144

Method to implement DestinationStyleOpInterface

mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
2166

this is a general folder that rewrites cast(dynamic) -> static

This revision is now accepted and ready to land.Jun 29 2023, 3:16 AM
qcolombet updated this revision to Diff 535702.Jun 29 2023, 3:29 AM
  • Remove redundant wrapper input()/output()
  • Update stall comment
  • Get rid of temporary variable op, emitOpError can be called directly
qcolombet marked 7 inline comments as done.Jun 29 2023, 3:31 AM
qcolombet added inline comments.
mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
125

Ah thanks, forgot to clean that out.
It made the porting from IREE easier.

144

Good catch

qcolombet updated this revision to Diff 535705.Jun 29 2023, 3:31 AM
  • Aligned ins and outs in test
  • Rebased
qcolombet marked an inline comment as done.Jun 29 2023, 3:32 AM

Should be fine for now. @chelini are you happy with this?

This revision was automatically updated to reflect the committed changes.

Should be fine for now. @chelini are you happy with this?

yes.