Validate Uniform decoration (#2181)
diff --git a/source/val/validate.h b/source/val/validate.h
index 6418839..fe357a2 100644
--- a/source/val/validate.h
+++ b/source/val/validate.h
@@ -131,7 +131,8 @@
/// Performs instruction validation.
spv_result_t InstructionPass(ValidationState_t& _, const Instruction* inst);
-/// Performs decoration validation.
+/// Performs decoration validation. Assumes each decoration on a group
+/// has been propagated down to the group members.
spv_result_t ValidateDecorations(ValidationState_t& _);
/// Performs validation of built-in variables.
diff --git a/source/val/validate_decorations.cpp b/source/val/validate_decorations.cpp
index c565890..f12c30f 100644
--- a/source/val/validate_decorations.cpp
+++ b/source/val/validate_decorations.cpp
@@ -968,77 +968,134 @@
return SPV_SUCCESS;
}
-spv_result_t CheckDecorationsOfConversions(ValidationState_t& vstate) {
- // Validates FPRoundingMode decoration for Shader Capabilities
- if (!vstate.HasCapability(SpvCapabilityShader)) return SPV_SUCCESS;
+// Returns SPV_SUCCESS if validation rules are satisfied for FPRoundingMode
+// decorations. Otherwise emits a diagnostic and returns something other than
+// SPV_SUCCESS.
+spv_result_t CheckFPRoundingModeForShaders(ValidationState_t& vstate,
+ const Instruction& inst) {
+ // Validates width-only conversion instruction for floating-point object
+ // i.e., OpFConvert
+ if (inst.opcode() != SpvOpFConvert) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+ << "FPRoundingMode decoration can be applied only to a "
+ "width-only conversion instruction for floating-point "
+ "object.";
+ }
+
+ // Validates Object operand of an OpStore
+ for (const auto& use : inst.uses()) {
+ const auto store = use.first;
+ if (store->opcode() != SpvOpStore) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+ << "FPRoundingMode decoration can be applied only to the "
+ "Object operand of an OpStore.";
+ }
+
+ if (use.second != 2) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+ << "FPRoundingMode decoration can be applied only to the "
+ "Object operand of an OpStore.";
+ }
+
+ const auto ptr_inst = vstate.FindDef(store->GetOperandAs<uint32_t>(0));
+ const auto ptr_type = vstate.FindDef(ptr_inst->GetOperandAs<uint32_t>(0));
+
+ const auto half_float_id = ptr_type->GetOperandAs<uint32_t>(2);
+ if (!vstate.IsFloatScalarOrVectorType(half_float_id) ||
+ vstate.GetBitWidth(half_float_id) != 16) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+ << "FPRoundingMode decoration can be applied only to the "
+ "Object operand of an OpStore storing through a pointer "
+ "to "
+ "a 16-bit floating-point scalar or vector object.";
+ }
+
+ // Validates storage class of the pointer to the OpStore
+ const auto storage = ptr_type->GetOperandAs<uint32_t>(1);
+ if (storage != SpvStorageClassStorageBuffer &&
+ storage != SpvStorageClassUniform &&
+ storage != SpvStorageClassPushConstant &&
+ storage != SpvStorageClassInput && storage != SpvStorageClassOutput) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+ << "FPRoundingMode decoration can be applied only to the "
+ "Object operand of an OpStore in the StorageBuffer, "
+ "Uniform, PushConstant, Input, or Output Storage "
+ "Classes.";
+ }
+ }
+ return SPV_SUCCESS;
+}
+
+// Returns SPV_SUCCESS if validation rules are satisfied for Uniform
+// decorations. Otherwise emits a diagnostic and returns something other than
+// SPV_SUCCESS. Assumes each decoration on a group has been propagated down to
+// the group members.
+spv_result_t CheckUniformDecoration(ValidationState_t& vstate,
+ const Instruction& inst,
+ const Decoration&) {
+ // Uniform must decorate an "object"
+ // - has a result ID
+ // - is an instantiation of a non-void type. So it has a type ID, and that
+ // type is not void.
+
+ // We already know the result ID is non-zero.
+
+ if (inst.type_id() == 0) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+ << "Uniform decoration applied to a non-object";
+ }
+ if (Instruction* type_inst = vstate.FindDef(inst.type_id())) {
+ if (type_inst->opcode() == SpvOpTypeVoid) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+ << "Uniform decoration applied to a value with void type";
+ }
+ } else {
+ // We might never get here because this would have been rejected earlier in
+ // the flow.
+ return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+ << "Uniform decoration applied to an object with invalid type";
+ }
+ return SPV_SUCCESS;
+}
+
+#define PASS_OR_BAIL_AT_LINE(X, LINE) \
+ { \
+ spv_result_t e##LINE = (X); \
+ if (e##LINE != SPV_SUCCESS) return e##LINE; \
+ }
+#define PASS_OR_BAIL(X) PASS_OR_BAIL_AT_LINE(X, __LINE__)
+
+// Check rules for decorations where we start from the decoration rather
+// than the decorated object. Assumes each decoration on a group have been
+// propagated down to the group members.
+spv_result_t CheckDecorationsFromDecoration(ValidationState_t& vstate) {
+ // Some rules are only checked for shaders.
+ const bool is_shader = vstate.HasCapability(SpvCapabilityShader);
for (const auto& kv : vstate.id_decorations()) {
const uint32_t id = kv.first;
const auto& decorations = kv.second;
- if (decorations.empty()) {
- continue;
- }
+ if (decorations.empty()) continue;
const Instruction* inst = vstate.FindDef(id);
assert(inst);
+ // We assume the decorations applied to a decoration group have already
+ // been propagated down to the group members.
+ if (inst->opcode() == SpvOpDecorationGroup) continue;
+
// Validates FPRoundingMode decoration
for (const auto& decoration : decorations) {
- if (decoration.dec_type() != SpvDecorationFPRoundingMode) {
- continue;
- }
-
- // Validates width-only conversion instruction for floating-point object
- // i.e., OpFConvert
- if (inst->opcode() != SpvOpFConvert) {
- return vstate.diag(SPV_ERROR_INVALID_ID, inst)
- << "FPRoundingMode decoration can be applied only to a "
- "width-only conversion instruction for floating-point "
- "object.";
- }
-
- // Validates Object operand of an OpStore
- for (const auto& use : inst->uses()) {
- const auto store = use.first;
- if (store->opcode() != SpvOpStore) {
- return vstate.diag(SPV_ERROR_INVALID_ID, inst)
- << "FPRoundingMode decoration can be applied only to the "
- "Object operand of an OpStore.";
- }
-
- if (use.second != 2) {
- return vstate.diag(SPV_ERROR_INVALID_ID, inst)
- << "FPRoundingMode decoration can be applied only to the "
- "Object operand of an OpStore.";
- }
-
- const auto ptr_inst = vstate.FindDef(store->GetOperandAs<uint32_t>(0));
- const auto ptr_type =
- vstate.FindDef(ptr_inst->GetOperandAs<uint32_t>(0));
-
- const auto half_float_id = ptr_type->GetOperandAs<uint32_t>(2);
- if (!vstate.IsFloatScalarOrVectorType(half_float_id) ||
- vstate.GetBitWidth(half_float_id) != 16) {
- return vstate.diag(SPV_ERROR_INVALID_ID, inst)
- << "FPRoundingMode decoration can be applied only to the "
- "Object operand of an OpStore storing through a pointer "
- "to "
- "a 16-bit floating-point scalar or vector object.";
- }
-
- // Validates storage class of the pointer to the OpStore
- const auto storage = ptr_type->GetOperandAs<uint32_t>(1);
- if (storage != SpvStorageClassStorageBuffer &&
- storage != SpvStorageClassUniform &&
- storage != SpvStorageClassPushConstant &&
- storage != SpvStorageClassInput &&
- storage != SpvStorageClassOutput) {
- return vstate.diag(SPV_ERROR_INVALID_ID, inst)
- << "FPRoundingMode decoration can be applied only to the "
- "Object operand of an OpStore in the StorageBuffer, "
- "Uniform, PushConstant, Input, or Output Storage "
- "Classes.";
- }
+ switch (decoration.dec_type()) {
+ case SpvDecorationFPRoundingMode:
+ if (is_shader)
+ PASS_OR_BAIL(CheckFPRoundingModeForShaders(vstate, *inst));
+ break;
+ case SpvDecorationUniform:
+ PASS_OR_BAIL(CheckUniformDecoration(vstate, *inst, decoration));
+ break;
+ default:
+ break;
}
}
}
@@ -1047,7 +1104,6 @@
} // namespace
-// Validates that decorations have been applied properly.
spv_result_t ValidateDecorations(ValidationState_t& vstate) {
if (auto error = CheckImportedVariableInitialization(vstate)) return error;
if (auto error = CheckDecorationsOfEntryPoints(vstate)) return error;
@@ -1055,7 +1111,7 @@
if (auto error = CheckLinkageAttrOfFunctions(vstate)) return error;
if (auto error = CheckVulkanMemoryModelDeprecatedDecorations(vstate))
return error;
- if (auto error = CheckDecorationsOfConversions(vstate)) return error;
+ if (auto error = CheckDecorationsFromDecoration(vstate)) return error;
return SPV_SUCCESS;
}
diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp
index a53418c..5db4445 100644
--- a/test/opt/aggressive_dead_code_elim_test.cpp
+++ b/test/opt/aggressive_dead_code_elim_test.cpp
@@ -5211,7 +5211,7 @@
TEST_F(AggressiveDCETest, PartiallyDeadGroupMemberDecorate) {
const std::string text = R"(
; CHECK: OpDecorate [[grp:%\w+]] Offset 0
-; CHECK: OpDecorate [[grp]] Uniform
+; CHECK: OpDecorate [[grp]] RelaxedPrecision
; CHECK: [[grp]] = OpDecorationGroup
; CHECK: OpGroupMemberDecorate [[grp]] [[output:%\w+]] 1
; CHECK: [[output]] = OpTypeStruct
@@ -5221,7 +5221,7 @@
OpEntryPoint Fragment %main "main" %output
OpExecutionMode %main OriginUpperLeft
OpDecorate %1 Offset 0
-OpDecorate %1 Uniform
+OpDecorate %1 RelaxedPrecision
%1 = OpDecorationGroup
OpGroupMemberDecorate %1 %var_struct 0 %output_struct 1
%void = OpTypeVoid
@@ -5250,7 +5250,7 @@
PartiallyDeadGroupMemberDecorateDifferentGroupDecorate) {
const std::string text = R"(
; CHECK: OpDecorate [[grp:%\w+]] Offset 0
-; CHECK: OpDecorate [[grp]] Uniform
+; CHECK: OpDecorate [[grp]] RelaxedPrecision
; CHECK: [[grp]] = OpDecorationGroup
; CHECK: OpGroupMemberDecorate [[grp]] [[output:%\w+]] 1
; CHECK-NOT: OpGroupMemberDecorate
@@ -5261,7 +5261,7 @@
OpEntryPoint Fragment %main "main" %output
OpExecutionMode %main OriginUpperLeft
OpDecorate %1 Offset 0
-OpDecorate %1 Uniform
+OpDecorate %1 RelaxedPrecision
%1 = OpDecorationGroup
OpGroupMemberDecorate %1 %var_struct 0
OpGroupMemberDecorate %1 %output_struct 1
diff --git a/test/val/val_capability_test.cpp b/test/val/val_capability_test.cpp
index f5650ab..488e957 100644
--- a/test/val/val_capability_test.cpp
+++ b/test/val/val_capability_test.cpp
@@ -1139,9 +1139,12 @@
"%intt = OpTypeInt 32 0\n" + std::string(kVoidFVoid),
AllCapabilities()),
std::make_pair(std::string(kOpenCLMemoryModel) +
+ // Uniform must target a non-void value.
"OpEntryPoint Kernel %func \"compute\" \n"
- "OpDecorate %intt Uniform\n"
- "%intt = OpTypeInt 32 0\n" + std::string(kVoidFVoid),
+ "OpDecorate %int0 Uniform\n"
+ "%intt = OpTypeInt 32 0\n" +
+ "%int0 = OpConstantNull %intt"
+ + std::string(kVoidFVoid),
ShaderDependencies()),
std::make_pair(std::string(kGLSL450MemoryModel) +
"OpEntryPoint Vertex %func \"shader\" \n"
diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp
index e9b748b..6db3d12 100644
--- a/test/val/val_decoration_test.cpp
+++ b/test/val/val_decoration_test.cpp
@@ -37,7 +37,7 @@
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpDecorate %1 ArrayStride 4
- OpDecorate %1 Uniform
+ OpDecorate %1 RelaxedPrecision
%2 = OpTypeFloat 32
%1 = OpTypeRuntimeArray %2
; Since %1 is used first in Decoration, it gets id 1.
@@ -49,7 +49,7 @@
EXPECT_THAT(
vstate_->id_decorations(id),
Eq(std::vector<Decoration>{Decoration(SpvDecorationArrayStride, {4}),
- Decoration(SpvDecorationUniform)}));
+ Decoration(SpvDecorationRelaxedPrecision)}));
}
TEST_F(ValidateDecorations, ValidateOpMemberDecorateRegistration) {
@@ -4510,6 +4510,86 @@
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
+// Uniform decoration
+
+TEST_F(ValidateDecorations, UniformDecorationGood) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+OpDecorate %int0 Uniform
+OpDecorate %var Uniform
+OpDecorate %val Uniform
+%void = OpTypeVoid
+%int = OpTypeInt 32 1
+%int0 = OpConstantNull %int
+%intptr = OpTypePointer Private %int
+%var = OpVariable %intptr Private
+%fn = OpTypeFunction %void
+%main = OpFunction %void None %fn
+%entry = OpLabel
+%val = OpLoad %int %var
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(), Eq(""));
+}
+
+TEST_F(ValidateDecorations, UniformDecorationTargetsTypeBad) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+OpDecorate %fn Uniform
+%void = OpTypeVoid
+%fn = OpTypeFunction %void
+%main = OpFunction %void None %fn
+%entry = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Uniform decoration applied to a non-object"));
+ EXPECT_THAT(getDiagnosticString(), HasSubstr("%2 = OpTypeFunction %void"));
+}
+
+TEST_F(ValidateDecorations, UniformDecorationTargetsVoidValueBad) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+OpName %call "call"
+OpName %myfunc "myfunc"
+OpDecorate %call Uniform
+%void = OpTypeVoid
+%fnty = OpTypeFunction %void
+%myfunc = OpFunction %void None %fnty
+%myfuncentry = OpLabel
+OpReturn
+OpFunctionEnd
+%main = OpFunction %void None %fnty
+%entry = OpLabel
+%call = OpFunctionCall %void %myfunc
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Uniform decoration applied to a value with void type\n"
+ " %call = OpFunctionCall %void %myfunc"));
+}
+
} // namespace
} // namespace val
} // namespace spvtools
diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
index 1c4fdd3..a162648 100644
--- a/test/val/val_id_test.cpp
+++ b/test/val/val_id_test.cpp
@@ -261,7 +261,7 @@
TEST_F(ValidateIdWithMessage, OpMemberDecorateGood) {
std::string spirv = kGLSL450MemoryModel + R"(
- OpMemberDecorate %2 0 Uniform
+ OpMemberDecorate %2 0 RelaxedPrecision
%1 = OpTypeInt 32 0
%2 = OpTypeStruct %1 %1)";
CompileSuccessfully(spirv.c_str());
@@ -269,7 +269,7 @@
}
TEST_F(ValidateIdWithMessage, OpMemberDecorateBad) {
std::string spirv = kGLSL450MemoryModel + R"(
- OpMemberDecorate %1 0 Uniform
+ OpMemberDecorate %1 0 RelaxedPrecision
%1 = OpTypeInt 32 0)";
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
@@ -279,7 +279,7 @@
}
TEST_F(ValidateIdWithMessage, OpMemberDecorateMemberBad) {
std::string spirv = kGLSL450MemoryModel + R"(
- OpMemberDecorate %1 3 Uniform
+ OpMemberDecorate %1 3 RelaxedPrecision
%int = OpTypeInt 32 0
%1 = OpTypeStruct %int %int)";
CompileSuccessfully(spirv.c_str());
@@ -293,7 +293,7 @@
TEST_F(ValidateIdWithMessage, OpGroupDecorateGood) {
std::string spirv = kGLSL450MemoryModel + R"(
%1 = OpDecorationGroup
- OpDecorate %1 Uniform
+ OpDecorate %1 RelaxedPrecision
OpDecorate %1 GLSLShared
OpGroupDecorate %1 %3 %4
%2 = OpTypeInt 32 0
@@ -305,7 +305,7 @@
TEST_F(ValidateIdWithMessage, OpDecorationGroupBad) {
std::string spirv = kGLSL450MemoryModel + R"(
%1 = OpDecorationGroup
- OpDecorate %1 Uniform
+ OpDecorate %1 RelaxedPrecision
OpDecorate %1 GLSLShared
OpMemberDecorate %1 0 Constant
)";
@@ -334,7 +334,7 @@
TEST_F(ValidateIdWithMessage, OpGroupDecorateTargetBad) {
std::string spirv = kGLSL450MemoryModel + R"(
%1 = OpDecorationGroup
- OpDecorate %1 Uniform
+ OpDecorate %1 RelaxedPrecision
OpDecorate %1 GLSLShared
OpGroupDecorate %1 %3
%2 = OpTypeInt 32 0)";
