This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add Acos, Asin, Atan, Sinh, Cosh, Pow to SPIRVGLSLOps
ClosedPublic

Authored by LiyangLingIntel on Sep 1 2020, 4:40 AM.

Diff Detail

Event Timeline

LiyangLingIntel created this revision.Sep 1 2020, 4:40 AM
LiyangLingIntel requested review of this revision.Sep 1 2020, 4:40 AM
mravishankar requested changes to this revision.Sep 1 2020, 10:31 AM

Thanks! Can we also add some tests for the operation here and serialization roundtrip tests here. This is in keeping with the procedure to add new op here

This revision now requires changes to proceed.Sep 1 2020, 10:31 AM

Thanks! Can we also add some tests for the operation here and serialization roundtrip tests here. This is in keeping with the procedure to add new op here

Sure, we are working on it.

[mlir][spirv] Add tests for SPIRVGLSLOps

antiagainst accepted this revision.Sep 2 2020, 8:34 AM
mravishankar accepted this revision.Sep 2 2020, 9:18 AM
This revision is now accepted and ready to land.Sep 2 2020, 9:18 AM

Hi!
It's glad to see this patch is accepted. As a first-time contributor, it seems I do not have the commit access to this project. Can you help me commit this change?

Done. Thanks for the contribution! :)

bondhugula added inline comments.
mlir/include/mlir/Dialect/SPIRV/SPIRVGLSLOps.td
268–278

A side question: These ops seem to be introducing a whole lot of duplication. Has SPIRV just considered using std dialect operations where they have the same semantics and are on the same operand types? sine, arcsine, tan, pow, exp, .. are all mathematically the same irrespective of which dialect they live in. If there are slight differences in semantics, a new op could be introduced briefly documenting the deviation. What are the benefits on creating <dialect_name>.op aliases? If there are no type conversions needed, why shouldn't/can't we mix std dialect ops?