spirv-val: Some Float16 fixes

[Snektron]

• Allows the Float16 capability from OpenCL 1.2. See here for docs.
• Enable ValidateSmallTypeUses for non-Shader modules
  • The rules for this are the same as for Shader modules
  • I only added a brief test here. Let me know if this should be tested more rigorously.

On a side note, it looks like clang does not generate the right capabilities here.

[CLAassistant]

All committers have signed the CLA.

[alan-baker]

Code-wise this seems ok, but I'd like Ben to comment on the actual OpenCL rules. The description of Float16Buffer is:

Allows a 16-bit OpTypeFloat instruction using the IEEE 754 encoding for creating an OpTypePointer to a 16-bit float. Pointers to a 16-bit float must not be dereferenced, unless specifically allowed by a specific instruction. All other uses of 16-bit OpTypeFloat are disallowed.

I'm not sure what is meant by "specifically allowed by a specific instruction" since I don't see any other mentions of Float16Buffer.

[Snektron]

I believe that part refers to the ability of some extensions to allow operation on fp16 types like https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/AMD/SPV_AMD_gpu_shader_half_float.asciidoc, though i guess this would technically already require Float16.

[bashbaug]

I'm not super familiar with this code, but looking at the tests it seems to be doing the right thing.

The Float16Buffer capability allows creating a pointer to a 16-bit float (half), and then use of those pointers with the vload_half and vstore_half family of extended instructions. I think this usage is handled by the check for IsPointerType in ValidateSmallTypeUses, so we're OK. Still, this might be an interesting test to add, to ensure this doesn't break in the future.

In other words, this should validate even though it doesn't declare Float16, because it declares Float16Buffer:

               OpCapability Addresses
               OpCapability Kernel
               OpCapability Float16Buffer
               OpMemoryModel Physical64 OpenCL
               OpEntryPoint Kernel %test "test"
       %void = OpTypeVoid
     %fp16_t = OpTypeFloat 16
%gptr_fp16_t = OpTypePointer CrossWorkgroup %fp16_t
 %kernel_sig = OpTypeFunction %void %gptr_fp16_t
       %test = OpFunction %void None %kernel_sig
        %ptr = OpFunctionParameter %gptr_fp16_t
      %entry = OpLabel
               OpReturn
               OpFunctionEnd

[alan-baker]

The Float16Buffer capability allows creating a pointer to a 16-bit float (half), and then use of those pointers with the vload_half and vstore_half family of extended instructions. I think this usage is handled by the check for IsPointerType in ValidateSmallTypeUses, so we're OK. Still, this might be an interesting test to add, to ensure this doesn't break in the future.

If that is true I'm not certain this is the right validation. This family of code checks the validation rules about 8- or 16-bit storage access capabilities. So for example, this would allow scalar loads of 16-bit floats. Based on @bashbaug comment this is not allowed.

[Snektron]

In other words, this should validate even though it doesn't declare Float16, because it declares Float16Buffer:

There is already such a test for the Shader capability (val_data_test.cpp, float16_good), though it only constructs the type and nothing else. I can add it no problem.

So for example, this would allow scalar loads of 16-bit floats.

Ah, I guess that is indeed missed right now. I'm slightly confused by this comment in ValidateSmallTypeUses though:

// The validator should previously have checked ways to generate 8- or 16-bit
// types. So we only need to considervalid paths from source to sink.
// When restricted, uses of 8- or 16-bit types can only be stores,
// width-only conversions, decorations and copy object.

I don't see why OpFConvert, OpUConvert, OpIConvert, and OpStore are actually allowed. Their instructions specifications do not list any exemptions for Float16 and in fact explicitly forbids dereferencing those pointers. (For Int8 and Int16 those rules don't apply anyway because you can't create the relevant type without the extension). What do you think @bashbaug @alan-baker?

There should probably also be a test using vload_half and vstore_half with only the Float16Buffer capability, and perhaps ValidateSmallTypeUses needs an exception for OpExtInst.

[alan-baker]

Those rules come from the following validation (and similar for 8-bit):

The capabilities StorageBuffer16BitAccess, UniformAndStorageBuffer16BitAccess, StoragePushConstant16, and StorageInputOutput16 do not generally add 16-bit operations. Rather, they add only the following specific abilities:

• An OpTypePointer pointing to a 16-bit scalar, a 16-bit vector, or a composite containing a 16-bit member can be used as the result type of OpVariable, or OpAccessChain, or OpInBoundsAccessChain.
• OpLoad can load 16-bit scalars, 16-bit vectors, and 16-bit matrices.
• OpStore can store 16-bit scalars, 16-bit vectors, and 16-bit matrices.
• OpCopyObject can be used for 16-bit scalars or composites containing 16-bit members.
• 16-bit scalars or 16-bit vectors can be used as operands to a width-only conversion instruction to another allowed type (OpFConvert, OpSConvert, or OpUConvert), and can be produced as results of a width-only conversion instruction from another allowed type.
• A structure containing a 16-bit member can be an operand to OpArrayLength.

In particular the conversions bullet.