This is an archive of the discontinued LLVM Phabricator instance.

[mlir][spirv] Support conversion of `CopySignOp` to spirv for 1D vector with 1 element
ClosedPublic

Authored by guraypp on Dec 7 2022, 2:35 AM.

Details

Summary

Conversion of CopySignOp to SPIRV is supported for scalar and vectors but not 1D vectors with 1 element (aka vector<1xf32>). This revisions adds supports this by treating them as scalars.

An alternative solution would be to allow 0D vectors for SPIRV, but the spec [0] strictly defines the vector type as non-0D.
"Vector: An ordered homogeneous collection of two or more scalars. Vector sizes are quite restrictive and dependent on the execution model."

[0] https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_types

Diff Detail

Event Timeline

guraypp created this revision.Dec 7 2022, 2:35 AM
guraypp requested review of this revision.Dec 7 2022, 2:35 AM
guraypp retitled this revision from [mlir][spirv] Convert `CopySignOp` with 0-D vector spirv to [mlir][spirv] Support conversion of `CopySignOp` to spirv for 0D vector .Dec 7 2022, 2:41 AM
ThomasRaoux added inline comments.Dec 7 2022, 8:48 AM
mlir/lib/Conversion/MathToSPIRV/MathToSPIRV.cpp
154–164

I think a simpler solution is to use the result of the type converter directly as it already knows to treat vector with 1 element as scalar. So I think you can just replace copySignOp.getType() here by type

guraypp updated this revision to Diff 480939.Dec 7 2022, 8:57 AM
guraypp retitled this revision from [mlir][spirv] Support conversion of `CopySignOp` to spirv for 0D vector to [mlir][spirv] Support conversion of `CopySignOp` to spirv for 0D vector.

address comments

mlir/lib/Conversion/MathToSPIRV/MathToSPIRV.cpp
154–164

That's a simple fix. Thanks for pointing that out!

ThomasRaoux accepted this revision.Dec 7 2022, 9:12 AM
This revision is now accepted and ready to land.Dec 7 2022, 9:12 AM
antiagainst added inline comments.Dec 7 2022, 9:21 AM
mlir/test/Conversion/MathToSPIRV/math-to-core-spirv.mlir
72

This is actually not 0-D vector? It's a 1-D 1-element vector?

guraypp retitled this revision from [mlir][spirv] Support conversion of `CopySignOp` to spirv for 0D vector to [mlir][spirv] Support conversion of `CopySignOp` to spirv for 1D vector with 1 element.Dec 8 2022, 12:05 AM
guraypp edited the summary of this revision. (Show Details)
guraypp added inline comments.Dec 8 2022, 12:12 AM
mlir/test/Conversion/MathToSPIRV/math-to-core-spirv.mlir
72

right, I altered change note accordingly. Thanks for pointing that!