mirror of
https://github.com/KhronosGroup/SPIRV-Tools
synced 2024-10-18 11:10:05 +00:00
Move token version/cap/ext checks from parsing to validation (#5370)
A token is allowed to parse even when it's from the wrong version, or is not enabled by a capability or extension. This allows more modules to parse. Version/capability/extension checking is fully moved to validation instead. Fixes: #5364
This commit is contained in:
parent
4788ff1578
commit
8e3da01b45
@ -21,6 +21,7 @@
|
||||
#include "source/ext_inst.h"
|
||||
#include "source/opcode.h"
|
||||
#include "source/operand.h"
|
||||
#include "source/spirv_target_env.h"
|
||||
#include "source/table.h"
|
||||
|
||||
namespace spvtools {
|
||||
@ -176,15 +177,18 @@ bool AssemblyGrammar::isValid() const {
|
||||
CapabilitySet AssemblyGrammar::filterCapsAgainstTargetEnv(
|
||||
const spv::Capability* cap_array, uint32_t count) const {
|
||||
CapabilitySet cap_set;
|
||||
const auto version = spvVersionForTargetEnv(target_env_);
|
||||
for (uint32_t i = 0; i < count; ++i) {
|
||||
spv_operand_desc cap_desc = {};
|
||||
spv_operand_desc entry = {};
|
||||
if (SPV_SUCCESS == lookupOperand(SPV_OPERAND_TYPE_CAPABILITY,
|
||||
static_cast<uint32_t>(cap_array[i]),
|
||||
&cap_desc)) {
|
||||
// spvOperandTableValueLookup() filters capabilities internally
|
||||
// according to the current target environment by itself. So we
|
||||
// should be safe to add this capability if the lookup succeeds.
|
||||
cap_set.insert(cap_array[i]);
|
||||
&entry)) {
|
||||
// This token is visible in this environment if it's in an appropriate
|
||||
// core version, or it is enabled by a capability or an extension.
|
||||
if ((version >= entry->minVersion && version <= entry->lastVersion) ||
|
||||
entry->numExtensions > 0u || entry->numCapabilities > 0u) {
|
||||
cap_set.insert(cap_array[i]);
|
||||
}
|
||||
}
|
||||
}
|
||||
return cap_set;
|
||||
|
@ -26,7 +26,6 @@
|
||||
#include "source/macro.h"
|
||||
#include "source/opcode.h"
|
||||
#include "source/spirv_constant.h"
|
||||
#include "source/spirv_target_env.h"
|
||||
|
||||
// For now, assume unified1 contains up to SPIR-V 1.3 and no later
|
||||
// SPIR-V version.
|
||||
@ -48,7 +47,7 @@ spv_result_t spvOperandTableGet(spv_operand_table* pOperandTable,
|
||||
return SPV_SUCCESS;
|
||||
}
|
||||
|
||||
spv_result_t spvOperandTableNameLookup(spv_target_env env,
|
||||
spv_result_t spvOperandTableNameLookup(spv_target_env,
|
||||
const spv_operand_table table,
|
||||
const spv_operand_type_t type,
|
||||
const char* name,
|
||||
@ -57,31 +56,18 @@ spv_result_t spvOperandTableNameLookup(spv_target_env env,
|
||||
if (!table) return SPV_ERROR_INVALID_TABLE;
|
||||
if (!name || !pEntry) return SPV_ERROR_INVALID_POINTER;
|
||||
|
||||
const auto version = spvVersionForTargetEnv(env);
|
||||
for (uint64_t typeIndex = 0; typeIndex < table->count; ++typeIndex) {
|
||||
const auto& group = table->types[typeIndex];
|
||||
if (type != group.type) continue;
|
||||
for (uint64_t index = 0; index < group.count; ++index) {
|
||||
const auto& entry = group.entries[index];
|
||||
// We consider the current operand as available as long as
|
||||
// 1. The target environment satisfies the minimal requirement of the
|
||||
// operand; or
|
||||
// 2. There is at least one extension enabling this operand; or
|
||||
// 3. There is at least one capability enabling this operand.
|
||||
//
|
||||
// Note that the second rule assumes the extension enabling this operand
|
||||
// is indeed requested in the SPIR-V code; checking that should be
|
||||
// validator's work.
|
||||
// it is in the grammar. It might not be *valid* to use,
|
||||
// but that should be checked by the validator, not by parsing.
|
||||
if (nameLength == strlen(entry.name) &&
|
||||
!strncmp(entry.name, name, nameLength)) {
|
||||
if ((version >= entry.minVersion && version <= entry.lastVersion) ||
|
||||
entry.numExtensions > 0u || entry.numCapabilities > 0u) {
|
||||
*pEntry = &entry;
|
||||
return SPV_SUCCESS;
|
||||
} else {
|
||||
// if there is no extension/capability then the version is wrong
|
||||
return SPV_ERROR_WRONG_VERSION;
|
||||
}
|
||||
*pEntry = &entry;
|
||||
return SPV_SUCCESS;
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -89,7 +75,7 @@ spv_result_t spvOperandTableNameLookup(spv_target_env env,
|
||||
return SPV_ERROR_INVALID_LOOKUP;
|
||||
}
|
||||
|
||||
spv_result_t spvOperandTableValueLookup(spv_target_env env,
|
||||
spv_result_t spvOperandTableValueLookup(spv_target_env,
|
||||
const spv_operand_table table,
|
||||
const spv_operand_type_t type,
|
||||
const uint32_t value,
|
||||
@ -110,33 +96,15 @@ spv_result_t spvOperandTableValueLookup(spv_target_env env,
|
||||
const auto beg = group.entries;
|
||||
const auto end = group.entries + group.count;
|
||||
|
||||
// We need to loop here because there can exist multiple symbols for the
|
||||
// same operand value, and they can be introduced in different target
|
||||
// environments, which means they can have different minimal version
|
||||
// requirements. For example, SubgroupEqMaskKHR can exist in any SPIR-V
|
||||
// version as long as the SPV_KHR_shader_ballot extension is there; but
|
||||
// starting from SPIR-V 1.3, SubgroupEqMask, which has the same numeric
|
||||
// value as SubgroupEqMaskKHR, is available in core SPIR-V without extension
|
||||
// requirements.
|
||||
// Assumes the underlying table is already sorted ascendingly according to
|
||||
// opcode value.
|
||||
const auto version = spvVersionForTargetEnv(env);
|
||||
for (auto it = std::lower_bound(beg, end, needle, comp);
|
||||
it != end && it->value == value; ++it) {
|
||||
// We consider the current operand as available as long as
|
||||
// 1. The target environment satisfies the minimal requirement of the
|
||||
// operand; or
|
||||
// 2. There is at least one extension enabling this operand; or
|
||||
// 3. There is at least one capability enabling this operand.
|
||||
//
|
||||
// Note that the second rule assumes the extension enabling this operand
|
||||
// is indeed requested in the SPIR-V code; checking that should be
|
||||
// validator's work.
|
||||
if ((version >= it->minVersion && version <= it->lastVersion) ||
|
||||
it->numExtensions > 0u || it->numCapabilities > 0u) {
|
||||
*pEntry = it;
|
||||
return SPV_SUCCESS;
|
||||
}
|
||||
// it is in the grammar. It might not be *valid* to use,
|
||||
// but that should be checked by the validator, not by parsing.
|
||||
*pEntry = it;
|
||||
return SPV_SUCCESS;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -18,7 +18,9 @@
|
||||
#include <vector>
|
||||
|
||||
#include "gmock/gmock.h"
|
||||
#include "source/assembly_grammar.h"
|
||||
#include "source/enum_set.h"
|
||||
#include "source/operand.h"
|
||||
#include "test/unit_spirv.h"
|
||||
|
||||
namespace spvtools {
|
||||
@ -31,12 +33,39 @@ using ::testing::TestWithParam;
|
||||
using ::testing::Values;
|
||||
using ::testing::ValuesIn;
|
||||
|
||||
// Emits a CapabilitySet to the given ostream, returning the ostream.
|
||||
inline std::ostream& operator<<(std::ostream& out, const CapabilitySet& cs) {
|
||||
out << "CapabilitySet{";
|
||||
auto ctx = spvContextCreate(SPV_ENV_UNIVERSAL_1_0);
|
||||
spvtools::AssemblyGrammar grammar(ctx);
|
||||
bool first = true;
|
||||
for (auto c : cs) {
|
||||
if (!first) {
|
||||
out << " ";
|
||||
first = false;
|
||||
}
|
||||
out << grammar.lookupOperandName(SPV_OPERAND_TYPE_CAPABILITY, uint32_t(c))
|
||||
<< "(" << uint32_t(c) << ")";
|
||||
}
|
||||
spvContextDestroy(ctx);
|
||||
out << "}";
|
||||
return out;
|
||||
}
|
||||
|
||||
// A test case for mapping an enum to a capability mask.
|
||||
struct EnumCapabilityCase {
|
||||
spv_operand_type_t type;
|
||||
uint32_t value;
|
||||
CapabilitySet expected_capabilities;
|
||||
};
|
||||
// Emits an EnumCapabilityCase to the ostream, returning the ostream.
|
||||
inline std::ostream& operator<<(std::ostream& out,
|
||||
const EnumCapabilityCase& ecc) {
|
||||
out << "EnumCapabilityCase{ " << spvOperandTypeStr(ecc.type) << "("
|
||||
<< unsigned(ecc.type) << "), " << ecc.value << ", "
|
||||
<< ecc.expected_capabilities << "}";
|
||||
return out;
|
||||
}
|
||||
|
||||
// Test fixture for testing EnumCapabilityCases.
|
||||
using EnumCapabilityTest =
|
||||
@ -56,7 +85,7 @@ TEST_P(EnumCapabilityTest, Sample) {
|
||||
|
||||
EXPECT_THAT(ElementsIn(cap_set),
|
||||
Eq(ElementsIn(std::get<1>(GetParam()).expected_capabilities)))
|
||||
<< " capability value " << std::get<1>(GetParam()).value;
|
||||
<< " enum value " << std::get<1>(GetParam()).value;
|
||||
spvContextDestroy(context);
|
||||
}
|
||||
|
||||
@ -417,7 +446,7 @@ INSTANTIATE_TEST_SUITE_P(
|
||||
|
||||
// See SPIR-V Section 3.20 Decoration
|
||||
INSTANTIATE_TEST_SUITE_P(
|
||||
Decoration, EnumCapabilityTest,
|
||||
Decoration_1_1, EnumCapabilityTest,
|
||||
Combine(Values(SPV_ENV_UNIVERSAL_1_0, SPV_ENV_UNIVERSAL_1_1),
|
||||
ValuesIn(std::vector<EnumCapabilityCase>{
|
||||
CASE1(DECORATION, Decoration::RelaxedPrecision, Shader),
|
||||
@ -469,6 +498,13 @@ INSTANTIATE_TEST_SUITE_P(
|
||||
CASE1(DECORATION, Decoration::Alignment, Kernel),
|
||||
})));
|
||||
|
||||
// See SPIR-V Section 3.20 Decoration
|
||||
INSTANTIATE_TEST_SUITE_P(Decoration_1_6, EnumCapabilityTest,
|
||||
Combine(Values(SPV_ENV_UNIVERSAL_1_6),
|
||||
ValuesIn(std::vector<EnumCapabilityCase>{
|
||||
CASE2(DECORATION, Decoration::Uniform,
|
||||
Shader, UniformDecoration)})));
|
||||
|
||||
#if 0
|
||||
// SpecId has different requirements in v1.0 and v1.1:
|
||||
INSTANTIATE_TEST_SUITE_P(DecorationSpecIdV10, EnumCapabilityTest,
|
||||
|
@ -5852,10 +5852,12 @@ TEST_F(ValidateImage, SignExtendV13Bad) {
|
||||
%res1 = OpImageRead %u32vec4 %img %u32vec2_01 SignExtend
|
||||
)";
|
||||
|
||||
EXPECT_THAT(CompileFailure(GenerateShaderCode(body, "", "Fragment", "",
|
||||
SPV_ENV_UNIVERSAL_1_3),
|
||||
SPV_ENV_UNIVERSAL_1_3, SPV_ERROR_WRONG_VERSION),
|
||||
HasSubstr("Invalid image operand 'SignExtend'"));
|
||||
CompileSuccessfully(
|
||||
GenerateShaderCode(body, "", "Fragment", "", SPV_ENV_UNIVERSAL_1_3));
|
||||
ASSERT_EQ(SPV_ERROR_WRONG_VERSION, ValidateInstructions());
|
||||
EXPECT_THAT(
|
||||
getDiagnosticString(),
|
||||
HasSubstr("SignExtend(4096) requires SPIR-V version 1.4 or later"));
|
||||
}
|
||||
|
||||
TEST_F(ValidateImage, ZeroExtendV13Bad) {
|
||||
@ -5864,10 +5866,12 @@ TEST_F(ValidateImage, ZeroExtendV13Bad) {
|
||||
%res1 = OpImageRead %u32vec4 %img %u32vec2_01 ZeroExtend
|
||||
)";
|
||||
|
||||
EXPECT_THAT(CompileFailure(GenerateShaderCode(body, "", "Fragment", "",
|
||||
SPV_ENV_UNIVERSAL_1_3),
|
||||
SPV_ENV_UNIVERSAL_1_3, SPV_ERROR_WRONG_VERSION),
|
||||
HasSubstr("Invalid image operand 'ZeroExtend'"));
|
||||
CompileSuccessfully(
|
||||
GenerateShaderCode(body, "", "Fragment", "", SPV_ENV_UNIVERSAL_1_3));
|
||||
ASSERT_EQ(SPV_ERROR_WRONG_VERSION, ValidateInstructions());
|
||||
EXPECT_THAT(
|
||||
getDiagnosticString(),
|
||||
HasSubstr("ZeroExtend(8192) requires SPIR-V version 1.4 or later"));
|
||||
}
|
||||
|
||||
TEST_F(ValidateImage, SignExtendScalarUIntTexelV14Good) {
|
||||
|
Loading…
Reference in New Issue
Block a user