This is an archive of the discontinued LLVM Phabricator instance.

[mlir][spirv] Add group ops
ClosedPublic

Authored by Hardcode84 on Nov 27 2022, 5:19 AM.

Details

Summary

Also, gen_spirv_dialect script was broken was broken due to SPV->SPIRV renamings.

Diff Detail

Event Timeline

Hardcode84 created this revision.Nov 27 2022, 5:19 AM
Hardcode84 requested review of this revision.Nov 27 2022, 5:19 AM

test roundtrip

antiagainst added inline comments.Nov 28 2022, 10:58 AM
mlir/include/mlir/Dialect/SPIRV/IR/SPIRVGroupOps.td
22

Can you update the summary and description to match the extension spec wording?

It's a bit annoying that the main SPIR-V spec only says "TBD" and no explanation so this is inheriting that. We'd prefer to have clear semantics here though. Diverging here is fine; the automatic importing tool is only meant for simplifying the process of defining new ops; some manual modification before/after is fine.

53

This and the result should be changed to SPIRV_ScalarOrVectorOf<SPIRV_Float>?

Similarly for the following places where we also use SPIRV_Scalar?

antiagainst requested changes to this revision.Nov 28 2022, 10:58 AM
This revision now requires changes to proceed.Nov 28 2022, 10:58 AM
Hardcode84 added inline comments.Nov 28 2022, 12:11 PM
mlir/include/mlir/Dialect/SPIRV/IR/SPIRVGroupOps.td
53

For non-KHR ops spec says

Result Type must be a 16-bit, 32-bit, or 64-bit floating-point type scalar.
The type of X must be the same as Result Type.

Not vector, so I'll change them to just SPIRV_Float/SPIRV_Integer

update types and description

Not vector, so I'll change them to just SPIRV_Float/SPIRV_Integer

Nevermind, looked in wrong place

Hardcode84 marked 2 inline comments as done.Nov 28 2022, 1:32 PM
antiagainst accepted this revision.Nov 28 2022, 2:13 PM
This revision is now accepted and ready to land.Nov 28 2022, 2:13 PM
This revision was automatically updated to reflect the committed changes.