Page MenuHomePhabricator

[mlir][spirv] Consolidate std.constant to spv.constant conversions
ClosedPublic

Authored by antiagainst on Mar 16 2020, 11:54 AM.

Details

Summary

This commit merges the DRR pattern for std.constant to spv.constant
conversion into the C++ OpConversionPattern. This allows us to have
remove the DRR pattern file. Along the way, this commit enhanced
std.constant to spv.constant conversion to consider type conversions,
which means converting the underlying attributes if necessary.

Depends On D76245

Diff Detail

Event Timeline

antiagainst created this revision.Mar 16 2020, 11:54 AM
mravishankar requested changes to this revision.Mar 17 2020, 9:14 AM
mravishankar added inline comments.
mlir/lib/Conversion/StandardToSPIRV/ConvertStandardToSPIRV.cpp
54

+1 for this is dangerous!

mlir/test/Conversion/StandardToSPIRV/std-to-spirv.mlir
308

How come these didnt become i32. I thought there is a vulkan extension needed for that.

This revision now requires changes to proceed.Mar 17 2020, 9:14 AM
antiagainst marked 4 inline comments as done.Mar 18 2020, 6:14 AM
antiagainst added inline comments.
mlir/lib/Conversion/StandardToSPIRV/ConvertStandardToSPIRV.cpp
54

Sadly although we say index cannot be negative, but AFAICT, we are actually relying on negative index values in passes like https://github.com/llvm/llvm-project/blob/bd0ca2627cfa1acde2a272347ed55d88a9751869/mlir/lib/Conversion/AffineToStandard/AffineToStandard.cpp#L126. That is the reason of this statement.

mlir/test/Conversion/StandardToSPIRV/std-to-spirv.mlir
308

It's governed by the Int16 capability, which is declared for this test block. i16 used for computation does not require extra extensions (well, if used in GLSL extended instructions it needs. If i16 is used in interface storage classes like StorageBuffer and others then we need extra extension, SPV_KHR_16bit_storage.

mravishankar accepted this revision.Mar 18 2020, 2:26 PM
This revision is now accepted and ready to land.Mar 18 2020, 2:26 PM
antiagainst marked 2 inline comments as done.Mar 18 2020, 2:27 PM
This revision was automatically updated to reflect the committed changes.