Skip to content

Commit

Permalink
spirv-val: Allow the ViewportIndex and Layer built-ins on SPIR-V 1.5 (#…
Browse files Browse the repository at this point in the history
…3986)

* spirv-val: Allow the ViewportIndex and Layer built-ins when their corresponding SPIR-V 1.5 capabilities are present
* Added tests for OpCapability ShaderViewportIndex and OpCapability ShaderLayer
  • Loading branch information
JustSid authored Oct 28, 2020
1 parent cbd1fa6 commit 5edb328
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 9 deletions.
21 changes: 17 additions & 4 deletions source/val/validate_builtins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

// Validates correctness of built-in variables.

#include "source/val/validate.h"

#include <functional>
#include <list>
#include <map>
Expand All @@ -33,6 +31,7 @@
#include "source/spirv_target_env.h"
#include "source/util/bitutils.h"
#include "source/val/instruction.h"
#include "source/val/validate.h"
#include "source/val/validation_state.h"

namespace spvtools {
Expand Down Expand Up @@ -2641,12 +2640,26 @@ spv_result_t BuiltInsValidator::ValidateLayerOrViewportIndexAtReference(
case SpvExecutionModelVertex:
case SpvExecutionModelTessellationEvaluation: {
if (!_.HasCapability(SpvCapabilityShaderViewportIndexLayerEXT)) {
if (operand == SpvBuiltInViewportIndex &&
_.HasCapability(SpvCapabilityShaderViewportIndex))
break; // Ok
if (operand == SpvBuiltInLayer &&
_.HasCapability(SpvCapabilityShaderLayer))
break; // Ok

const char* capability = "ShaderViewportIndexLayerEXT";

if (operand == SpvBuiltInViewportIndex)
capability = "ShaderViewportIndexLayerEXT or ShaderViewportIndex";
if (operand == SpvBuiltInLayer)
capability = "ShaderViewportIndexLayerEXT or ShaderLayer";

return _.diag(SPV_ERROR_INVALID_DATA, &referenced_from_inst)
<< "Using BuiltIn "
<< _.grammar().lookupOperandName(SPV_OPERAND_TYPE_BUILT_IN,
operand)
<< " in Vertex or Tessellation execution model requires "
"the ShaderViewportIndexLayerEXT capability.";
<< " in Vertex or Tessellation execution model requires the "
<< capability << " capability.";
}
break;
}
Expand Down
68 changes: 63 additions & 5 deletions test/val/val_builtins_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ using ValidateVulkanCombineBuiltInExecutionModelDataTypeCapabilityExtensionResul
std::tuple<const char*, const char*, const char*, const char*,
const char*, const char*, const char*, TestResult>>;

using ValidateGenericCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult =
spvtest::ValidateBase<std::tuple<spv_target_env, const char*, const char*,
const char*, const char*, const char*,
const char*, const char*, TestResult>>;

bool InitializerRequired(spv_target_env env, const char* const storage_class) {
return spvIsWebGPUEnv(env) && (strncmp(storage_class, "Output", 6) == 0 ||
strncmp(storage_class, "Private", 7) == 0 ||
Expand Down Expand Up @@ -258,6 +263,36 @@ TEST_P(
}
}

TEST_P(
ValidateGenericCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult,
InMain) {
const spv_target_env env = std::get<0>(GetParam());
const char* const built_in = std::get<1>(GetParam());
const char* const execution_model = std::get<2>(GetParam());
const char* const storage_class = std::get<3>(GetParam());
const char* const data_type = std::get<4>(GetParam());
const char* const capabilities = std::get<5>(GetParam());
const char* const extensions = std::get<6>(GetParam());
const char* const vuid = std::get<7>(GetParam());
const TestResult& test_result = std::get<8>(GetParam());

CodeGenerator generator =
GetInMainCodeGenerator(env, built_in, execution_model, storage_class,
capabilities, extensions, data_type);

CompileSuccessfully(generator.Build(), env);
ASSERT_EQ(test_result.validation_result, ValidateInstructions(env));
if (test_result.error_str) {
EXPECT_THAT(getDiagnosticString(), HasSubstr(test_result.error_str));
}
if (test_result.error_str2) {
EXPECT_THAT(getDiagnosticString(), HasSubstr(test_result.error_str2));
}
if (vuid) {
EXPECT_THAT(getDiagnosticString(), AnyVUID(vuid));
}
}

CodeGenerator GetInFunctionCodeGenerator(spv_target_env env,
const char* const built_in,
const char* const execution_model,
Expand Down Expand Up @@ -1207,14 +1242,21 @@ INSTANTIATE_TEST_SUITE_P(
"Geometry, or Fragment execution models"))));

INSTANTIATE_TEST_SUITE_P(
LayerAndViewportIndexExecutionModelEnabledByCapability,
ViewportIndexExecutionModelEnabledByCapability,
ValidateVulkanCombineBuiltInExecutionModelDataTypeResult,
Combine(Values("Layer", "ViewportIndex"),
Values("Vertex", "TessellationEvaluation"), Values("Output"),
Values("%u32"), Values(nullptr),
Combine(Values("ViewportIndex"), Values("Vertex", "TessellationEvaluation"),
Values("Output"), Values("%u32"), Values(nullptr),
Values(TestResult(
SPV_ERROR_INVALID_DATA,
"requires the ShaderViewportIndexLayerEXT capability"))));
"ShaderViewportIndexLayerEXT or ShaderViewportIndex"))));

INSTANTIATE_TEST_SUITE_P(
LayerExecutionModelEnabledByCapability,
ValidateVulkanCombineBuiltInExecutionModelDataTypeResult,
Combine(Values("Layer"), Values("Vertex", "TessellationEvaluation"),
Values("Output"), Values("%u32"), Values(nullptr),
Values(TestResult(SPV_ERROR_INVALID_DATA,
"ShaderViewportIndexLayerEXT or ShaderLayer"))));

INSTANTIATE_TEST_SUITE_P(
LayerAndViewportIndexFragmentNotInput,
Expand Down Expand Up @@ -1260,6 +1302,22 @@ INSTANTIATE_TEST_SUITE_P(
"needs to be a 32-bit int scalar",
"has bit width 64"))));

INSTANTIATE_TEST_SUITE_P(
LayerCapability,
ValidateGenericCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult,
Combine(Values(SPV_ENV_VULKAN_1_2), Values("Layer"), Values("Vertex"),
Values("Output"), Values("%u32"),
Values("OpCapability ShaderLayer\n"), Values(nullptr),
Values(nullptr), Values(TestResult())));

INSTANTIATE_TEST_SUITE_P(
ViewportIndexCapability,
ValidateGenericCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult,
Combine(Values(SPV_ENV_VULKAN_1_2), Values("ViewportIndex"),
Values("Vertex"), Values("Output"), Values("%u32"),
Values("OpCapability ShaderViewportIndex\n"), Values(nullptr),
Values(nullptr), Values(TestResult())));

INSTANTIATE_TEST_SUITE_P(
PatchVerticesSuccess,
ValidateVulkanCombineBuiltInExecutionModelDataTypeResult,
Expand Down

0 comments on commit 5edb328

Please sign in to comment.