This is an archive of the discontinued LLVM Phabricator instance.

[mlir][tosa] Add accumulator type attribute to TOSA dialect
ClosedPublic

Authored by tatwaichong on Mar 17 2023, 11:13 AM.

Details

Summary

Tosa supports fp16 and fp32 accumulator type for fp16 input, but
no way to tell for computational operators whether accumulator
should be fp16 or fp32 from input type. Add this new attribute
to specify the type.

Set to fp32 by default for now. When the time fp16 is supported,
the accumulator type can be selected based on trade-off between
performance and accuracy.

Diff Detail

Event Timeline

tatwaichong created this revision.Mar 17 2023, 11:13 AM
tatwaichong requested review of this revision.Mar 17 2023, 11:13 AM
tatwaichong edited the summary of this revision. (Show Details)
tatwaichong edited the summary of this revision. (Show Details)Mar 17 2023, 11:17 AM

Could you include the corresponding change in TosaToLinalg? It should be a fairly trivial change.

Rebase onto top-of-tree to include the fix that isn't relevant to this patch.

Read the acc type attr in TosaToLinalg.

Rebase to include OUTSIDE fix for bazel build error.

Good spot. Thanks for the reminder. I missed TosaToLinalg part. Reading from the acctype attr is complemented in newer patch. Do I still miss something necessary else?

Rebase to include bazel fix for transform dialect (not relevant to this patch)

Look like the recent build of mlir repo isn't stable.
Have to be careful to pick a commit to rebase.
Now rebase this PR to a commit that can pass bazel build test locally.

rsuderman added inline comments.Mar 22 2023, 6:53 PM
mlir/include/mlir/Dialect/Tosa/IR/TosaOps.td
91

Rather than doing an enum for the accumulator type you should be able to use a TypeAttr. Then in a separate verifier you can check that the subtype is correct. It will make the TosaToLinalg pass cleaner as you won't need a switch on the enum.

Define the new accumulator type as TypeAttr rather than EnumAttr.

tatwaichong marked an inline comment as done.Mar 30 2023, 10:53 PM
tatwaichong added inline comments.
mlir/include/mlir/Dialect/Tosa/IR/TosaOps.td
91

I realize this is a awesome suggestion. It makes the change become intuitive and simple.

jpienaar accepted this revision.Apr 19 2023, 9:31 PM
This revision is now accepted and ready to land.Apr 19 2023, 9:31 PM