SPIR-V validation error: "Interface variable is used by entry point, but is not listed as an interface"

[karolherbst]

Multiple SyCL CTS tests are triggering the "Interface variable is used by entry point, but is not listed as an interface" spir-v validation error, e.g. test_atomic_fence

; SPIR-V
; Version: 1.4
; Generator: Khronos LLVM/SPIR-V Translator; 14
; Bound: 1113
; Schema: 0
...
               OpEntryPoint Kernel %23 "_ZTSZZN17atomic_fence_test16run_atomic_fenceISt17integral_constantIN4sycl3_V112memory_orderELS4_3EES1_INS3_12memory_scopeELS6_2EES1_INS_9test_typeELS8_1EEEclERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESI_SI_ENKUlRNS3_7handlerEE_clESK_EUlNS3_7nd_itemILi1EEEE_"
...
    %__spirv_BuiltInGlobalOffset = OpVariable %_ptr_CrossWorkgroup_v3ulong CrossWorkgroup
...
         %23 = OpFunction %void None %22
...
         %65 = OpLoad %v3ulong %__spirv_BuiltInGlobalOffset Aligned 32

From the SPIR-V specification on OpEntryPoint:

"Interface is a list of of global OpVariable instructions. These declare the set of global variables from a module that form the interface of this entry point. The set of Interface must be equal to or a superset of the global OpVariable Result referenced by the entry point’s static call tree, within the interface’s storage classes. Before version 1.4, the interface’s storage classes are limited to the Input and Output storage classes. Starting with version 1.4, the interface’s storage classes are all storage classes used in declaring all global variables referenced by the entry point’s call tree."

Side Note: the storage class of builtins also have to be in the Input storage class according to the OpenCL SPIR-V Env specification in 2.9: "An OpVariable in a SPIR-V module with the BuiltIn decoration represents a built-in variable. All built-in variables must be in the Input storage class."

Side Note2: I'm confused this even comes up, as this issue was fixed years ago in the SPIRV-LLVM-Translator:

• Add the input storage class and use it for builtins instead of Uniform KhronosGroup/SPIRV-LLVM-Translator#144
• Encode and decode Input and Output Variables used in entry points KhronosGroup/SPIRV-LLVM-Translator#178

[karolherbst]

I actually filed a bug already a few months ago on the "Built In variable" storage class: #6703

[asudarsa]

This is the llvm-spirv command I run on the bc file attached. This llvm-spirv is built using Khronos main branch SPIRV-LLVM-Translator.

llvm-spirv -o atomic_fence.spv -spirv-max-version=1.4 -spirv-debug-info-version=ocl-100 -spirv-allow-extra-diexpressions -spirv-allow-unknown-intrinsics=llvm.genx. -spirv-ext=-all,+SPV_EXT_shader_atomic_float_add,+SPV_EXT_shader_atomic_float_min_max,+SPV_KHR_no_integer_wrap_decoration,+SPV_KHR_float_controls,+SPV_KHR_expect_assume,+SPV_KHR_linkonce_odr,+SPV_INTEL_subgroups,+SPV_INTEL_media_block_io,+SPV_INTEL_device_side_avc_motion_estimation,+SPV_INTEL_fpga_loop_controls,+SPV_INTEL_unstructured_loop_controls,+SPV_INTEL_fpga_reg,+SPV_INTEL_blocking_pipes,+SPV_INTEL_function_pointers,+SPV_INTEL_kernel_attributes,+SPV_INTEL_io_pipes,+SPV_INTEL_inline_assembly,+SPV_INTEL_arbitrary_precision_integers,+SPV_INTEL_float_controls2,+SPV_INTEL_vector_compute,+SPV_INTEL_fast_composite,+SPV_INTEL_joint_matrix,+SPV_INTEL_arbitrary_precision_fixed_point,+SPV_INTEL_arbitrary_precision_floating_point,+SPV_INTEL_variable_length_array,+SPV_INTEL_fp_fast_math_mode,+SPV_INTEL_long_constant_composite,+SPV_INTEL_arithmetic_fence,+SPV_INTEL_global_variable_decorations,+SPV_INTEL_fpga_buffer_location,+SPV_INTEL_fpga_argument_interfaces,+SPV_INTEL_fpga_invocation_pipelining_attributes,+SPV_INTEL_optnone,+SPV_INTEL_token_type,+SPV_INTEL_bfloat16_conversion,+SPV_INTEL_joint_matrix,+SPV_INTEL_hw_thread_queries,+SPV_INTEL_memory_access_aliasing,+SPV_KHR_uniform_group_instructions,+SPV_INTEL_masked_gather_scatter,+SPV_INTEL_tensor_float32_conversion atomic_fence.bc

I get the following OpEntryPoint in the generated spv code. Looks like all the interfaces are listed as we expect.

OpEntryPoint Kernel %1356 "_ZTSZZN17atomic_fence_test16run_atomic_fenceISt17integral_constantIN4sycl3_V112memory_orderELS4_3EES1_INS3_12memory_scopeELS6_2EES1_INS_9test_typeELS8_1EEEclERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESI_SI_ENKUlRNS3_7handlerEE_clESK_EUlNS3_7nd_itemILi1EEEE_" %__spirv_BuiltInGlobalOffset %__spirv_BuiltInGlobalInvocationId %__spirv_BuiltInLocalInvocationId %__spirv_BuiltInWorkgroupId %__spirv_BuiltInGlobalLinearId %__spirv_BuiltInWorkgroupSize

However, I see a new error now:

error: line 68: AliasDomainDeclINTEL must appear in a block
  %61 = OpAliasDomainDeclINTEL

Will open a separate issue, may be.

Thanks

[asudarsa]

@karolherbst

Can you please check when convenient?
I did verify that the EntryPoint did not have interfaces listed with an older llvm-spirv.

Thanks

[karolherbst]

@karolherbst

Can you please check when convenient? I did verify that the EntryPoint did not have interfaces listed with an older llvm-spirv.

Thanks

this isn't a bug inside llvm-spirv, because I've fixed it there a long time ago. This is a bug inside Intel's SyCL implementation, which I have no idea why it happens, but it does.

[asudarsa]

@karolherbst
Can you please check when convenient? I did verify that the EntryPoint did not have interfaces listed with an older llvm-spirv.
Thanks

this isn't a bug inside llvm-spirv, because I've fixed it there a long time ago. This is a bug inside Intel's SyCL implementation, which I have no idea why it happens, but it does.

Ah..of course..thanks for the redirect. Given that it's reported here, I should have tried llvm-spirv from intel/llvm branch. I will try that and see if I am able to reproduce the error.

Thanks

[asudarsa]

This issue is mainly due to differences between intel/llvm's llvm-spirv and khronos. We are tracking the effort to minimize this difference here: #7592

In this particular case, we are missing the following commit in intel/llvm:
commit 352ea14d320da10fcf72c19b46c50020e657c89a
Author: KornevNikita [email protected]
Date: Tue Dec 21 16:07:34 2021 +0300
Fix the collection of entry point interfaces (#1334)
This is a patch to expand the collection of entry point interfaces.
In SPIR-V 1.4 and later OpEntryPoint must list all global variables in the
interface. Also fix quoted string output in SPIRV text format.
Co-authored-by: Alexey Sotkin [email protected]

Reintroducing this commit resolves this issue. We will investigate further and provide a PR to intel/llvm soon.

Thanks

[karolherbst]

This issue is mainly due to differences between intel/llvm's llvm-spirv and khronos. We are tracking the effort to minimize this difference here: #7592

In this particular case, we are missing the following commit in intel/llvm: commit 352ea14d320da10fcf72c19b46c50020e657c89a Author: KornevNikita [email protected] Date: Tue Dec 21 16:07:34 2021 +0300 Fix the collection of entry point interfaces (#1334) This is a patch to expand the collection of entry point interfaces. In SPIR-V 1.4 and later OpEntryPoint must list all global variables in the interface. Also fix quoted string output in SPIRV text format. Co-authored-by: Alexey Sotkin [email protected]

Reintroducing this commit resolves this issue. We will investigate further and provide a PR to intel/llvm soon.

Thanks

Though is 352ea14d320da10fcf72c19b46c50020e657c89a enough as the storage class of builtins needs to be Input?

I don't think spirv-val validates this yet as this is mainly an OpenCL SPIR-V Env spec thing.

[asudarsa]

Hi @karolherbst

Thanks again for reporting this. A fix has been merged for this. Can you please verify if this issue is resolved at your end?

Sincerely

[asudarsa]

I do see the error reported by spirv-val goes away with this patch.
For this issue and other similar SYCL-CTS/spirv-val fails, it will be helpful if we can know the reproduction steps that you used in order to reproduce the error on our end. I might have missed something in my local test.

Thanks again
Sincerely

[karolherbst]

spirv-val doesn't report any error if builtins aren't in the Input storage class, but it is a OpenCL SPIR-V Env spec violation nonetheless.

[karolherbst]

Anyway, this can be closed as we already have a bug for the invalid storage class: #6703