Add validation for SPV_EXT_fragment_shader_interlock (#2650)
diff --git a/Android.mk b/Android.mk
index 06d8811..8e44378 100644
--- a/Android.mk
+++ b/Android.mk
@@ -62,6 +62,7 @@
source/val/validate_instruction.cpp \
source/val/validate_memory.cpp \
source/val/validate_memory_semantics.cpp \
+ source/val/validate_misc.cpp \
source/val/validate_mode_setting.cpp \
source/val/validate_layout.cpp \
source/val/validate_literals.cpp \
diff --git a/BUILD.gn b/BUILD.gn
index 5c6f311..fb9e8a4 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -422,6 +422,7 @@
"source/val/validate_logicals.cpp",
"source/val/validate_memory.cpp",
"source/val/validate_memory_semantics.cpp",
+ "source/val/validate_misc.cpp",
"source/val/validate_mode_setting.cpp",
"source/val/validate_non_uniform.cpp",
"source/val/validate_primitives.cpp",
diff --git a/DEPS b/DEPS
index 3c5361f..9a8e922 100644
--- a/DEPS
+++ b/DEPS
@@ -11,7 +11,7 @@
'googletest_revision': '98a0d007d7092b72eea0e501bb9ad17908a1a036',
'testing_revision': '340252637e2e7c72c0901dcbeeacfff419e19b59',
're2_revision': '6cf8ccd82dbaab2668e9b13596c68183c9ecd13f',
- 'spirv_headers_revision': 'e74c389f81915d0a48d6df1af83c3862c5ad85ab',
+ 'spirv_headers_revision': '8b911bd2ba37677037b38c9bd286c7c05701bcda',
}
deps = {
diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
index 93938f0..bd4c465 100644
--- a/source/CMakeLists.txt
+++ b/source/CMakeLists.txt
@@ -299,6 +299,7 @@
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_logicals.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_memory.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_memory_semantics.cpp
+ ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_misc.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_mode_setting.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_non_uniform.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_primitives.cpp
diff --git a/source/val/function.cpp b/source/val/function.cpp
index 90893b3..af1498d 100644
--- a/source/val/function.cpp
+++ b/source/val/function.cpp
@@ -389,5 +389,29 @@
return return_value;
}
+bool Function::CheckLimitations(const ValidationState_t& _,
+ const Function* entry_point,
+ std::string* reason) const {
+ bool return_value = true;
+ std::stringstream ss_reason;
+
+ for (const auto& is_compatible : limitations_) {
+ std::string message;
+ if (!is_compatible(_, entry_point, &message)) {
+ if (!reason) return false;
+ return_value = false;
+ if (!message.empty()) {
+ ss_reason << message << "\n";
+ }
+ }
+ }
+
+ if (!return_value && reason) {
+ *reason = ss_reason.str();
+ }
+
+ return return_value;
+}
+
} // namespace val
} // namespace spvtools
diff --git a/source/val/function.h b/source/val/function.h
index 9cda2ff..0d6873d 100644
--- a/source/val/function.h
+++ b/source/val/function.h
@@ -216,6 +216,16 @@
execution_model_limitations_.push_back(is_compatible);
}
+ /// Registers limitation with an |is_compatible| functor.
+ void RegisterLimitation(std::function<bool(const ValidationState_t& _,
+ const Function*, std::string*)>
+ is_compatible) {
+ limitations_.push_back(is_compatible);
+ }
+
+ bool CheckLimitations(const ValidationState_t& _, const Function* entry_point,
+ std::string* reason) const;
+
/// Returns true if the given execution model passes the limitations stored in
/// execution_model_limitations_. Returns false otherwise and fills optional
/// |reason| parameter.
@@ -376,6 +386,12 @@
std::list<std::function<bool(SpvExecutionModel, std::string*)>>
execution_model_limitations_;
+ /// Stores limitations imposed by instructions used within the function.
+ /// Similar to execution_model_limitations_;
+ std::list<std::function<bool(const ValidationState_t& _, const Function*,
+ std::string*)>>
+ limitations_;
+
/// Stores ids of all functions called from this function.
std::set<uint32_t> function_call_targets_;
};
diff --git a/source/val/validate.cpp b/source/val/validate.cpp
index 4a730da..0840662 100644
--- a/source/val/validate.cpp
+++ b/source/val/validate.cpp
@@ -366,7 +366,7 @@
// Keep these passes in the order they appear in the SPIR-V specification
// sections to maintain test consistency.
- // Miscellaneous
+ if (auto error = MiscPass(*vstate, &instruction)) return error;
if (auto error = DebugPass(*vstate, &instruction)) return error;
if (auto error = AnnotationPass(*vstate, &instruction)) return error;
if (auto error = ExtensionPass(*vstate, &instruction)) return error;
diff --git a/source/val/validate.h b/source/val/validate.h
index fe357a2..aaae570 100644
--- a/source/val/validate.h
+++ b/source/val/validate.h
@@ -199,6 +199,9 @@
/// Validates correctness of function instructions.
spv_result_t FunctionPass(ValidationState_t& _, const Instruction* inst);
+/// Validates correctness of miscellaneous instructions.
+spv_result_t MiscPass(ValidationState_t& _, const Instruction* inst);
+
/// Validates execution limitations.
///
/// Verifies execution models are allowed for all functionality they contain.
diff --git a/source/val/validate_builtins.cpp b/source/val/validate_builtins.cpp
index bcd691f..568d729 100644
--- a/source/val/validate_builtins.cpp
+++ b/source/val/validate_builtins.cpp
@@ -2810,7 +2810,11 @@
case SpvBuiltInWorldToObjectNV:
case SpvBuiltInHitTNV:
case SpvBuiltInHitKindNV:
- case SpvBuiltInIncomingRayFlagsNV: {
+ case SpvBuiltInIncomingRayFlagsNV:
+ case SpvBuiltInWarpsPerSMNV:
+ case SpvBuiltInSMCountNV:
+ case SpvBuiltInWarpIDNV:
+ case SpvBuiltInSMIDNV: {
// No validation rules (for the moment).
break;
}
diff --git a/source/val/validate_execution_limitations.cpp b/source/val/validate_execution_limitations.cpp
index d449307..aac1c49 100644
--- a/source/val/validate_execution_limitations.cpp
+++ b/source/val/validate_execution_limitations.cpp
@@ -53,6 +53,17 @@
}
}
}
+
+ std::string reason;
+ if (!func->CheckLimitations(_, _.function(entry_id), &reason)) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "OpEntryPoint Entry Point <id> '" << _.getIdName(entry_id)
+ << "'s callgraph contains function <id> "
+ << _.getIdName(inst->id())
+ << ", which cannot be used with the current execution "
+ "modes:\n"
+ << reason;
+ }
}
return SPV_SUCCESS;
}
diff --git a/source/val/validate_misc.cpp b/source/val/validate_misc.cpp
new file mode 100644
index 0000000..dcc65f3
--- /dev/null
+++ b/source/val/validate_misc.cpp
@@ -0,0 +1,81 @@
+// Copyright (c) 2018 Google LLC.
+// Copyright (c) 2019 NVIDIA Corporation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "source/val/validate.h"
+
+#include "source/opcode.h"
+#include "source/spirv_target_env.h"
+#include "source/val/instruction.h"
+#include "source/val/validation_state.h"
+
+namespace spvtools {
+namespace val {
+
+spv_result_t MiscPass(ValidationState_t& _, const Instruction* inst) {
+ switch (inst->opcode()) {
+ case SpvOpBeginInvocationInterlockEXT:
+ case SpvOpEndInvocationInterlockEXT:
+ _.function(inst->function()->id())
+ ->RegisterExecutionModelLimitation(
+ SpvExecutionModelFragment,
+ "OpBeginInvocationInterlockEXT/OpEndInvocationInterlockEXT "
+ "require Fragment execution model");
+
+ _.function(inst->function()->id())
+ ->RegisterLimitation([](const ValidationState_t& state,
+ const Function* entry_point,
+ std::string* message) {
+ const auto* execution_modes =
+ state.GetExecutionModes(entry_point->id());
+
+ auto find_interlock = [](const SpvExecutionMode& mode) {
+ switch (mode) {
+ case SpvExecutionModePixelInterlockOrderedEXT:
+ case SpvExecutionModePixelInterlockUnorderedEXT:
+ case SpvExecutionModeSampleInterlockOrderedEXT:
+ case SpvExecutionModeSampleInterlockUnorderedEXT:
+ case SpvExecutionModeShadingRateInterlockOrderedEXT:
+ case SpvExecutionModeShadingRateInterlockUnorderedEXT:
+ return true;
+ default:
+ return false;
+ }
+ };
+
+ bool found = false;
+ if (execution_modes) {
+ auto i = std::find_if(execution_modes->begin(),
+ execution_modes->end(), find_interlock);
+ found = (i != execution_modes->end());
+ }
+
+ if (!found) {
+ *message =
+ "OpBeginInvocationInterlockEXT/OpEndInvocationInterlockEXT "
+ "require a fragment shader interlock execution mode.";
+ return false;
+ }
+ return true;
+ });
+ break;
+ default:
+ break;
+ }
+
+ return SPV_SUCCESS;
+}
+
+} // namespace val
+} // namespace spvtools
diff --git a/source/val/validate_mode_setting.cpp b/source/val/validate_mode_setting.cpp
index 943629d..cbcf11a 100644
--- a/source/val/validate_mode_setting.cpp
+++ b/source/val/validate_mode_setting.cpp
@@ -90,6 +90,26 @@
"one of DepthGreater, DepthLess or DepthUnchanged "
"execution modes.";
}
+ if (execution_modes &&
+ 1 < std::count_if(
+ execution_modes->begin(), execution_modes->end(),
+ [](const SpvExecutionMode& mode) {
+ switch (mode) {
+ case SpvExecutionModePixelInterlockOrderedEXT:
+ case SpvExecutionModePixelInterlockUnorderedEXT:
+ case SpvExecutionModeSampleInterlockOrderedEXT:
+ case SpvExecutionModeSampleInterlockUnorderedEXT:
+ case SpvExecutionModeShadingRateInterlockOrderedEXT:
+ case SpvExecutionModeShadingRateInterlockUnorderedEXT:
+ return true;
+ default:
+ return false;
+ }
+ })) {
+ return _.diag(SPV_ERROR_INVALID_DATA, inst)
+ << "Fragment execution model entry points can specify at most "
+ "one fragment shader interlock execution mode.";
+ }
break;
case SpvExecutionModelTessellationControl:
case SpvExecutionModelTessellationEvaluation:
@@ -376,6 +396,12 @@
case SpvExecutionModeDepthGreater:
case SpvExecutionModeDepthLess:
case SpvExecutionModeDepthUnchanged:
+ case SpvExecutionModePixelInterlockOrderedEXT:
+ case SpvExecutionModePixelInterlockUnorderedEXT:
+ case SpvExecutionModeSampleInterlockOrderedEXT:
+ case SpvExecutionModeSampleInterlockUnorderedEXT:
+ case SpvExecutionModeShadingRateInterlockOrderedEXT:
+ case SpvExecutionModeShadingRateInterlockUnorderedEXT:
if (!std::all_of(models->begin(), models->end(),
[](const SpvExecutionModel& model) {
return model == SpvExecutionModelFragment;
diff --git a/test/val/val_modes_test.cpp b/test/val/val_modes_test.cpp
index bc157a4..c5b1a37 100644
--- a/test/val/val_modes_test.cpp
+++ b/test/val/val_modes_test.cpp
@@ -1002,6 +1002,105 @@
"constant instructions."));
}
+TEST_F(ValidateMode, FragmentShaderInterlockVertexBad) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability FragmentShaderPixelInterlockEXT
+OpExtension "SPV_EXT_fragment_shader_interlock"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Vertex %main "main"
+OpExecutionMode %main PixelInterlockOrderedEXT
+)" + kVoidFunction;
+
+ CompileSuccessfully(spirv);
+ EXPECT_THAT(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+ EXPECT_THAT(
+ getDiagnosticString(),
+ HasSubstr(
+ "Execution mode can only be used with the Fragment execution model"));
+}
+
+TEST_F(ValidateMode, FragmentShaderInterlockTooManyModesBad) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability FragmentShaderPixelInterlockEXT
+OpCapability FragmentShaderSampleInterlockEXT
+OpExtension "SPV_EXT_fragment_shader_interlock"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpExecutionMode %main PixelInterlockOrderedEXT
+OpExecutionMode %main SampleInterlockOrderedEXT
+)" + kVoidFunction;
+
+ CompileSuccessfully(spirv);
+ EXPECT_THAT(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+ EXPECT_THAT(
+ getDiagnosticString(),
+ HasSubstr("Fragment execution model entry points can specify at most "
+ "one fragment shader interlock execution mode"));
+}
+
+TEST_F(ValidateMode, FragmentShaderInterlockNoModeBad) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability FragmentShaderPixelInterlockEXT
+OpExtension "SPV_EXT_fragment_shader_interlock"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+%void = OpTypeVoid
+%void_fn = OpTypeFunction %void
+%func = OpFunction %void None %void_fn
+%entryf = OpLabel
+OpBeginInvocationInterlockEXT
+OpEndInvocationInterlockEXT
+OpReturn
+OpFunctionEnd
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+%1 = OpFunctionCall %void %func
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_THAT(SPV_ERROR_INVALID_ID, ValidateInstructions());
+ EXPECT_THAT(
+ getDiagnosticString(),
+ HasSubstr(
+ "OpBeginInvocationInterlockEXT/OpEndInvocationInterlockEXT require a "
+ "fragment shader interlock execution mode"));
+}
+
+TEST_F(ValidateMode, FragmentShaderInterlockGood) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability FragmentShaderPixelInterlockEXT
+OpExtension "SPV_EXT_fragment_shader_interlock"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpExecutionMode %main PixelInterlockOrderedEXT
+%void = OpTypeVoid
+%void_fn = OpTypeFunction %void
+%func = OpFunction %void None %void_fn
+%entryf = OpLabel
+OpBeginInvocationInterlockEXT
+OpEndInvocationInterlockEXT
+OpReturn
+OpFunctionEnd
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+%1 = OpFunctionCall %void %func
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_THAT(SPV_SUCCESS, ValidateInstructions());
+}
+
} // namespace
} // namespace val
} // namespace spvtools
