encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

spirv-reader: Restrict use of ConstOffset 

Only permitted for image sampling (and later gather).

Only permitted for 2D, 2D Array, and 3D textures

Fixed: tint:408
Change-Id: Ib97bd17e45046ec8a2147b46899bf52ad9a8f883
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35980
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
This commit is contained in:
David Neto 2020-12-17 20:32:12 +00:00 committed by Commit Bot service account
parent 3ec1d5eae7
commit 9a644c7903
2 changed files with 91 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/reader/spirv/function.cc
View File

@ -500,6 +500,22 @@ bool IsSampledImageAccess(SpvOp opcode) {
return false;
}
// @param opcode a SPIR-V opcode
// @returns true if the given instruction is an image sampling operation.
bool IsImageSampling(SpvOp opcode) {
switch (opcode) {
case SpvOpImageSampleImplicitLod:
case SpvOpImageSampleExplicitLod:
case SpvOpImageSampleDrefImplicitLod:
case SpvOpImageSampleDrefExplicitLod:
return true;
default:
// WGSL doesn't have *Proj* texturing.
break;
}
return false;
}
// @param opcode a SPIR-V opcode
// @returns true if the given instruction is an image access instruction
// whose first input operand is an OpImage value.
@ -3989,7 +4005,8 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) {
params.push_back(create<ast::IdentifierExpression>(
Source{}, ast_module_.RegisterSymbol(name), name));
if (IsSampledImageAccess(inst.opcode())) {
const auto opcode = inst.opcode();
if (IsSampledImageAccess(opcode)) {
// Form the sampler operand.
const auto* sampler = parser_impl_.GetMemoryObjectDeclarationForHandle(
image_or_sampled_image_operand_id, false);
@ -4029,7 +4046,7 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) {
std::string builtin_name;
bool use_load_suffix = true;
switch (inst.opcode()) {
switch (opcode) {
case SpvOpImageSampleImplicitLod:
case SpvOpImageSampleExplicitLod:
builtin_name = "textureSample";
@ -4121,12 +4138,28 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) {
}
if (arg_index < num_args &&
(image_operands_mask & SpvImageOperandsConstOffsetMask)) {
if (!IsImageSampling(opcode)) {
return Fail() << "ConstOffset is only permitted for sampling operations: "
<< inst.PrettyPrint();
}
switch (texture_type->dim()) {
case ast::type::TextureDimension::k2d:
case ast::type::TextureDimension::k2dArray:
case ast::type::TextureDimension::k3d:
break;
default:
return Fail() << "ConstOffset is only permitted for 2D, 2D Arrayed, "
"and 3D textures: "
<< inst.PrettyPrint();
}
params.push_back(ToSignedIfUnsigned(MakeOperand(inst, arg_index)).expr);
image_operands_mask ^= SpvImageOperandsConstOffsetMask;
arg_index++;
}
if (arg_index < num_args &&
(image_operands_mask & SpvImageOperandsSampleMask)) {
// TODO(dneto): only permitted with ImageFetch
params.push_back(ToI32(MakeOperand(inst, arg_index)).expr);
image_operands_mask ^= SpvImageOperandsSampleMask;
arg_index++;
@ -4184,7 +4217,7 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) {
value = create<ast::BitcastExpression>(Source{}, result_type, call_expr);
}
if (!expected_component_type->Is<ast::type::F32>() &&
IsSampledImageAccess(inst.opcode())) {
IsSampledImageAccess(opcode)) {
// WGSL permits sampled image access only on float textures.
// Reject this case in the SPIR-V reader, at least until SPIR-V validation
// catches up with this rule and can reject it earlier in the workflow.

137
src/reader/spirv/parser_impl_handle_test.cc
View File

@ -30,6 +30,7 @@ namespace {
using ::testing::Eq;
using ::testing::HasSubstr;
using ::testing::Not;
using ::testing::StartsWith;
std::string Preamble() {
return R"(
@ -2617,28 +2618,6 @@ INSTANTIATE_TEST_SUITE_P(
Identifier[not set]{vi12}
Identifier[not set]{vf1234}
)
})"},
// OpImageWrite with ConstOffset
{"%float 2D 0 0 0 2 Rgba32f",
"OpImageWrite %im %vi12 %vf1234 ConstOffset %offsets2d",
R"(
Variable{
Decorations{
SetDecoration{2}
BindingDecoration{1}
}
x_20
uniform_constant
__storage_texture_write_only_2d_rgba32float
})",
R"(Call[not set]{
Identifier[not set]{textureStore}
(
Identifier[not set]{x_20}
Identifier[not set]{vi12}
Identifier[not set]{vf1234}
Identifier[not set]{offsets2d}
)
})"}}));
INSTANTIATE_TEST_SUITE_P(
@ -3085,35 +3064,6 @@ INSTANTIATE_TEST_SUITE_P(
}
}
}
})"},
// OpImageRead with ConstOffset
{"%float 2D 0 0 0 2 Rgba32f",
"%99 = OpImageRead %v4float %im %vi12 ConstOffset %offsets2d",
R"(Variable{
Decorations{
SetDecoration{2}
BindingDecoration{1}
}
x_20
uniform_constant
__storage_texture_read_only_2d_rgba32float
})",
R"(VariableDeclStatement{
VariableConst{
x_99
none
__vec_4__f32
{
Call[not set]{
Identifier[not set]{textureLoad}
(
Identifier[not set]{x_20}
Identifier[not set]{vi12}
Identifier[not set]{offsets2d}
)
}
}
}
})"}}));
INSTANTIATE_TEST_SUITE_P(
@ -3146,36 +3096,6 @@ INSTANTIATE_TEST_SUITE_P(
}
}
}
})"},
// OpImageFetch with ConstOffset
// TODO(dneto): Seems this is not valid in WGSL.
{"%float 2D 0 0 0 1 Unknown",
"%99 = OpImageFetch %v4float %im %vi12 ConstOffset %offsets2d",
R"(Variable{
Decorations{
SetDecoration{2}
BindingDecoration{1}
}
x_20
uniform_constant
__sampled_texture_2d__f32
})",
R"(VariableDeclStatement{
VariableConst{
x_99
none
__vec_4__f32
{
Call[not set]{
Identifier[not set]{textureLoad}
(
Identifier[not set]{x_20}
Identifier[not set]{vi12}
Identifier[not set]{offsets2d}
)
}
}
}
})"},
// OpImageFetch with explicit level
{"%float 2D 0 0 0 1 Unknown",
@ -3760,7 +3680,7 @@ TEST_P(SpvParserTest_ImageCoordsTest, MakeCoordinateOperandsForImageAccess) {
)";
auto p = parser(test::Assemble(assembly));
if (!p->BuildAndParseInternalModule()) {
EXPECT_THAT(p->error(), Eq(GetParam().expected_error)) << assembly;
EXPECT_THAT(p->error(), StartsWith(GetParam().expected_error)) << assembly;
} else {
EXPECT_TRUE(p->error().empty()) << p->error();
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
@ -4456,6 +4376,59 @@ INSTANTIATE_TEST_SUITE_P(
"sampled image must have float component type",
{}}}));
INSTANTIATE_TEST_SUITE_P(
ConstOffset_BadInstruction_Errors,
SpvParserTest_ImageCoordsTest,
::testing::ValuesIn(std::vector<ImageCoordsCase>{
// ImageFetch
{"%uint 2D 0 0 0 1 Unknown",
"%result = OpImageFetch %v4uint %sampled_image %vf12 ConstOffset "
"%the_vu12",
"ConstOffset is only permitted for sampling operations: ",
{}},
// ImageRead
{"%uint 2D 0 0 0 2 Rgba32ui",
"%result = OpImageRead %v4uint %im %vu12 ConstOffset %the_vu12",
"ConstOffset is only permitted for sampling operations: ",
{}},
// ImageWrite
{"%uint 2D 0 0 0 2 Rgba32ui",
"OpImageWrite %im %vu12 %vu1234 ConstOffset %the_vu12",
"ConstOffset is only permitted for sampling operations: ",
{}}
// TODO(dneto): Gather
// TODO(dneto): DrefGather
}));
INSTANTIATE_TEST_SUITE_P(
ConstOffset_BadDim_Errors,
SpvParserTest_ImageCoordsTest,
::testing::ValuesIn(std::vector<ImageCoordsCase>{
// 1D
{"%uint 1D 0 0 0 1 Unknown",
"%result = OpImageSampleImplicitLod %v4float %sampled_image %vf1234 "
"ConstOffset %the_vu12",
"ConstOffset is only permitted for 2D, 2D Arrayed, and 3D textures: ",
{}},
// 1D Array
{"%uint 1D 0 1 0 1 Unknown",
"%result = OpImageSampleImplicitLod %v4float %sampled_image %vf1234 "
"ConstOffset %the_vu12",
"ConstOffset is only permitted for 2D, 2D Arrayed, and 3D textures: ",
{}},
// Cube
{"%uint Cube 0 0 0 1 Unknown",
"%result = OpImageSampleImplicitLod %v4float %sampled_image %vf1234 "
"ConstOffset %the_vu12",
"ConstOffset is only permitted for 2D, 2D Arrayed, and 3D textures: ",
{}},
// Cube Array
{"%uint Cube 0 1 0 1 Unknown",
"%result = OpImageSampleImplicitLod %v4float %sampled_image %vf1234 "
"ConstOffset %the_vu12",
"ConstOffset is only permitted for 2D, 2D Arrayed, and 3D textures: ",
{}}}));
} // namespace
} // namespace spirv
} // namespace reader