how best to add validation for Kernel SPIR-V

[bashbaug]

I'd like to start adding some basic validation checks that are missing for Kernel SPIR-V and I am looking for guidance how best to do this. If it is helpful to provide a specific example, I would like to check that all built-in variables are in the Input storage class and that they all have the expected type, see: https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_Env.html#_built_in_variables

One of the first issues I'm finding is that a lot of this type of validation is only enabled for Vulkan environments. For example:

SPIRV-Tools/source/val/validate_builtins.cpp

Lines 4280 to 4293 in 9bd44d0

	spv_result_t BuiltInsValidator::ValidateSingleBuiltInAtDefinition(
	const Decoration& decoration, const Instruction& inst) {
	const spv::BuiltIn label = spv::BuiltIn(decoration.params()[0]);
	if (!spvIsVulkanEnv(_.context()->target_env)) {
	// Early return. All currently implemented rules are based on Vulkan spec.
	//
	// TODO: If you are adding validation rules for environments other than
	// Vulkan (or general rules which are not environment independent), then
	// you need to modify or remove this condition. Consider also adding early
	// returns into BuiltIn-specific rules, so that the system doesn't spawn new
	// rules which don't do anything.
	return SPV_SUCCESS;
	}

Would it be best to keep the current BuiltIn-specific rules as-is, call them only for Vulkan, and define other rules for Kernel SPIR-V? Or, would it be best to call the same BuiltIn-specific rules for both Vulkan and Kernel SPIR-V and to update them to handle both types of SPIR-V?

Thanks in advance for any advice you can provide!

[alan-baker]

Reusing the infrastructure is preferable for both builtins and general validation. I could see adding a generic environment based filter before validating any particular builtin.

For general validation the checks should be based on the appropriate requirement (e.g. either HasCapability(Kernel) or spvEnvIsOpenCL(env)).