Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn off ClipDistance CullDistance cap checks for Vulkan #262

Closed
wants to merge 1 commit into from
Closed

Turn off ClipDistance CullDistance cap checks for Vulkan #262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ Revision history for SPIRV-Tools

v2016.1-dev 2016-07-04
- Start v2016.1
- Fix https://github.com/KhronosGroup/SPIRV-Tools/issues/261
Turn off ClipDistance and CullDistance capability checks for Vulkan.

v2016.0 2016-07-04

Expand Down
24 changes: 20 additions & 4 deletions source/validate_instruction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,27 @@ spv_result_t CapabilityError(ValidationState_t& _, int which_operand,
spv_capability_mask_t RequiredCapabilities(const AssemblyGrammar& grammar,
spv_operand_type_t type,
uint32_t operand) {
spv_capability_mask_t result = 0;
spv_operand_desc operand_desc;
if (SPV_SUCCESS == grammar.lookupOperand(type, operand, &operand_desc))
return operand_desc->capabilities;
else
return 0;

if (SPV_SUCCESS == grammar.lookupOperand(type, operand, &operand_desc)) {
result = operand_desc->capabilities;

// There's disagreement about whether mere mention of ClipDistance and
// CullDistance implies a requirement to declare their associated
// capabilities. Until the dust settles, turn off those checks.
// See https://github.com/KhronosGroup/SPIRV-Tools/issues/261
// TODO(dneto): Once the final decision is made, fix this in a more
// permanent way, e.g. by generating Vulkan-specific operand tables that
// eliminate this capability dependency.
if (type == SPV_OPERAND_TYPE_BUILT_IN &&
grammar.target_env() == SPV_ENV_VULKAN_1_0) {
result = result & (~(SPV_CAPABILITY_AS_MASK(SpvCapabilityClipDistance) |
SPV_CAPABILITY_AS_MASK(SpvCapabilityCullDistance)));
}
}

return result;
}

} // namespace
Expand Down
32 changes: 32 additions & 0 deletions test/Validate.Capability.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ using ValidateCapability = spvtest::ValidateBase<CapTestParameter>;
// Always assembles using v1.1.
using ValidateCapabilityV11 = spvtest::ValidateBase<CapTestParameter>;

// Always assembles using Vulkan 1.0.
// TODO(dneto): Refactor all these tests to scale better across environments.
using ValidateCapabilityVulkan10 = spvtest::ValidateBase<CapTestParameter>;

TEST_F(ValidateCapability, Default) {
const char str[] = R"(
OpCapability Kernel
Expand Down Expand Up @@ -1057,6 +1061,23 @@ make_pair(string(kOpenCLMemoryModel) +
"%intt = OpTypeInt 32 1\n", ShaderDependencies())
)),);

// There's disagreement about whether mere mention of ClipDistance and
// CullDistance implies a requirement to declare their associated capabilities.
// Until the dust settles, turn off those checks.
// See https://github.com/KhronosGroup/SPIRV-Tools/issues/261
INSTANTIATE_TEST_CASE_P(BuiltIn, ValidateCapabilityVulkan10,
Combine(
// Vulkan 1.0 is based on SPIR-V 1.0
ValuesIn(AllV10Capabilities()),
Values(
make_pair(string(kGLSL450MemoryModel) +
"OpDecorate %intt BuiltIn ClipDistance\n"
"%intt = OpTypeInt 32 1\n", AllV10Capabilities()),
make_pair(string(kGLSL450MemoryModel) +
"OpDecorate %intt BuiltIn CullDistance\n"
"%intt = OpTypeInt 32 1\n", AllV10Capabilities())
)),);

// TODO(umar): Selection Control
// TODO(umar): Loop Control
// TODO(umar): Function Control
Expand Down Expand Up @@ -1134,6 +1155,8 @@ bool Exists(const std::string& capability, spv_target_env env) {
capability.size(), &dummy);
}



TEST_P(ValidateCapability, Capability) {
const string capability = Capability(GetParam());
spv_target_env env =
Expand All @@ -1153,6 +1176,15 @@ TEST_P(ValidateCapabilityV11, Capability) {
<< test_code;
}

TEST_P(ValidateCapabilityVulkan10, Capability) {
const string test_code = MakeAssembly(GetParam());
std::cout << test_code << std::endl;
CompileSuccessfully(test_code, SPV_ENV_VULKAN_1_0);
ASSERT_EQ(ExpectedResult(GetParam()),
ValidateInstructions(SPV_ENV_VULKAN_1_0))
<< test_code;
}

TEST_F(ValidateCapability, SemanticsIdIsAnIdNotALiteral) {
// From https://github.com/KhronosGroup/SPIRV-Tools/issues/248
// The validator was interpreting the memory semantics ID number
Expand Down