KhronosGroup / KhronosGroup/SPIRV-Tools

spirv-val accepts an OpenCL.std printf format operand typed as a pointer to an array of i8

Open
#6,846 0 comments 0 reactions 0 assignees View on GitHub
component:validation
Dominant language
C++
Stars
1.4k
Forks
709
Avg merge
1d 22h
Merged PRs (30d)
28

Description

`spirv-val` accepts an `OpenCL.std printf` whose `format` operand is typed `OpTypePointer UniformConstant [N x i8]`.

The OpenCL Extended Instruction Set Specification 1.00 revision 9 states, in the `printf` entry (`KhronosGroup/SPIRV-Docs`, `specs/OpenCLGenerated.adoc`, included by `specs/OpenCL.std.adoc`):

```
_format_ must be a _pointer_(_constant_) to _i8_.
```

Reproducer:

```
OpCapability Kernel
OpCapability Addresses
OpCapability Int8
%1 = OpExtInstImport "OpenCL.std"
OpMemoryModel Physical64 OpenCL
OpEntryPoint Kernel %k "k"
%void = OpTypeVoid
%fnty = OpTypeFunction %void
%uint = OpTypeInt 32 0
%uchar = OpTypeInt 8 0
%uint_3 = OpConstant %uint 3
%arr3 = OpTypeArray %uchar %uint_3
%ptr_arr3 = OpTypePointer UniformConstant %arr3
%uchar_104 = OpConstant %uchar 104
%uchar_105 = OpConstant %uchar 105
%uchar_0 = OpConstant %uchar 0
%init = OpConstantComposite %arr3 %uchar_104 %uchar_105 %uchar_0
%fmt = OpVariable %ptr_arr3 UniformConstant %init
%k = OpFunction %void None %fnty
%entry = OpLabel
%r = OpExtInst %uint %1 printf %fmt
OpReturn
OpFunctionEnd
```

```
$ spirv-as --target-env spv1.0 repro.spvasm -o repro.spv
$ for env in spv1.0 opencl1.2 opencl2.0 opencl2.2; do
spirv-val --target-env $env repro.spv; echo "$env rc=$?"
done
spv1.0 rc=0
opencl1.2 rc=0
opencl2.0 rc=0
opencl2.2 rc=0
```

The check itself does run. Changing the element type from `i8` to `i32` is rejected as expected:

```
error: line 16: OpenCL.std printf: expected Format data type to be 8-bit int
```

The array is unwrapped before the element check, in `source/val/validate_extensions.cpp`, `case OpenCLLIB::Printf:`

```cpp
// If pointer points to an array, get the type of an element
if (_.IsIntArrayType(format_data_type))
format_data_type = _.GetComponentType(format_data_type);
```

That unwrap was added deliberately in #5677 to accept `llvm-spirv` output (fixes intel/llvm#11733), so the current behaviour may well be intended. Filing because the validator and the specification now disagree, and a producer emitting the array form gets no signal from `spirv-val` that it has left what the spec text permits. Consumers are then free to differ on whether to accept it.

Either resolution would close the gap: relax the specification wording to permit a pointer to an array of `i8`, or have `spirv-val` diagnose the array form.

Contributor guide

Open the contributing guide

Research direction

Start with the OpenCLLIB::Printf case in source/val/validate_extensions.cpp and the printf entry in specs/OpenCLGenerated.adoc. Run the supplied spirv-as and spirv-val reproducer across the listed environments, then determine whether the specification or validator should change. Done means the specification and validation behavior agree for the pointer-to-array form.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.