spirv-fuzz: Always add new globals to entry point interfaces (#3113)
In the context of SPIR-V 1.4 or higher, global variables cannot be
used by an instruction unless they are listed in the interface of all
entry points that might invoke the instruction. This change
conservatively adds new global variables to the interfaces of all
entry points (if the SPIR-V version is 1.4 or higher).
Issue #3111 notes that a more rigorous approach to entry point
interfaces could be taken in spirv-fuzz, which would allow being less
conservative here.
diff --git a/source/fuzz/transformation_add_global_variable.cpp b/source/fuzz/transformation_add_global_variable.cpp
index 83d3589..cea268c 100644
--- a/source/fuzz/transformation_add_global_variable.cpp
+++ b/source/fuzz/transformation_add_global_variable.cpp
@@ -83,6 +83,22 @@
MakeUnique<opt::Instruction>(context, SpvOpVariable, message_.type_id(),
message_.fresh_id(), input_operands));
fuzzerutil::UpdateModuleIdBound(context, message_.fresh_id());
+
+ if (PrivateGlobalsMustBeDeclaredInEntryPointInterfaces(context)) {
+ // Conservatively add this global to the interface of every entry point in
+ // the module. This means that the global is available for other
+ // transformations to use.
+ //
+ // A downside of this is that the global will be in the interface even if it
+ // ends up never being used.
+ //
+ // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3111) revisit
+ // this if a more thorough approach to entry point interfaces is taken.
+ for (auto& entry_point : context->module()->entry_points()) {
+ entry_point.AddOperand({SPV_OPERAND_TYPE_ID, {message_.fresh_id()}});
+ }
+ }
+
// We have added an instruction to the module, so need to be careful about the
// validity of existing analyses.
context->InvalidateAnalysesExceptFor(opt::IRContext::Analysis::kAnalysisNone);
@@ -94,5 +110,22 @@
return result;
}
+bool TransformationAddGlobalVariable::
+ PrivateGlobalsMustBeDeclaredInEntryPointInterfaces(
+ opt::IRContext* context) {
+ // TODO(afd): We capture the universal environments for which this requirement
+ // holds. The check should be refined on demand for other target
+ // environments.
+ switch (context->grammar().target_env()) {
+ case SPV_ENV_UNIVERSAL_1_0:
+ case SPV_ENV_UNIVERSAL_1_1:
+ case SPV_ENV_UNIVERSAL_1_2:
+ case SPV_ENV_UNIVERSAL_1_3:
+ return false;
+ default:
+ return true;
+ }
+}
+
} // namespace fuzz
} // namespace spvtools
diff --git a/source/fuzz/transformation_add_global_variable.h b/source/fuzz/transformation_add_global_variable.h
index ed1cb12..ca63e68 100644
--- a/source/fuzz/transformation_add_global_variable.h
+++ b/source/fuzz/transformation_add_global_variable.h
@@ -48,6 +48,9 @@
protobufs::Transformation ToMessage() const override;
private:
+ static bool PrivateGlobalsMustBeDeclaredInEntryPointInterfaces(
+ opt::IRContext* context);
+
protobufs::TransformationAddGlobalVariable message_;
};
diff --git a/test/fuzz/transformation_add_global_variable_test.cpp b/test/fuzz/transformation_add_global_variable_test.cpp
index 6bcc5e2..eda6828 100644
--- a/test/fuzz/transformation_add_global_variable_test.cpp
+++ b/test/fuzz/transformation_add_global_variable_test.cpp
@@ -52,7 +52,7 @@
OpFunctionEnd
)";
- const auto env = SPV_ENV_UNIVERSAL_1_4;
+ const auto env = SPV_ENV_UNIVERSAL_1_3;
const auto consumer = nullptr;
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
ASSERT_TRUE(IsValid(env, context.get()));
@@ -167,6 +167,109 @@
ASSERT_TRUE(IsEqual(env, after_transformation, context.get()));
}
+TEST(TransformationAddGlobalVariableTest, TestEntryPointInterfaceEnlargement) {
+ // This checks that when global variables are added to a SPIR-V 1.4+ module,
+ // they are also added to entry points of that module.
+ std::string shader = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %4 "m1"
+ OpEntryPoint Vertex %5 "m2"
+ OpExecutionMode %4 OriginUpperLeft
+ OpSource ESSL 310
+ %2 = OpTypeVoid
+ %3 = OpTypeFunction %2
+ %6 = OpTypeFloat 32
+ %7 = OpTypeInt 32 1
+ %8 = OpTypeVector %6 2
+ %9 = OpTypePointer Function %6
+ %10 = OpTypePointer Private %6
+ %20 = OpTypePointer Uniform %6
+ %11 = OpTypePointer Function %7
+ %12 = OpTypePointer Private %7
+ %13 = OpTypePointer Private %8
+ %14 = OpVariable %10 Private
+ %15 = OpVariable %20 Uniform
+ %16 = OpConstant %7 1
+ %17 = OpTypePointer Private %10
+ %18 = OpTypeBool
+ %19 = OpTypePointer Private %18
+ %21 = OpConstantTrue %18
+ %4 = OpFunction %2 None %3
+ %30 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ %5 = OpFunction %2 None %3
+ %31 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ )";
+
+ const auto env = SPV_ENV_UNIVERSAL_1_4;
+ const auto consumer = nullptr;
+ const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
+ ASSERT_TRUE(IsValid(env, context.get()));
+
+ FactManager fact_manager;
+
+ TransformationAddGlobalVariable transformations[] = {
+ // %100 = OpVariable %12 Private
+ TransformationAddGlobalVariable(100, 12, 0),
+
+ // %101 = OpVariable %12 Private %16
+ TransformationAddGlobalVariable(101, 12, 16),
+
+ // %102 = OpVariable %19 Private %21
+ TransformationAddGlobalVariable(102, 19, 21)};
+
+ for (auto& transformation : transformations) {
+ ASSERT_TRUE(transformation.IsApplicable(context.get(), fact_manager));
+ transformation.Apply(context.get(), &fact_manager);
+ }
+ ASSERT_TRUE(IsValid(env, context.get()));
+
+ std::string after_transformation = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %4 "m1" %100 %101 %102
+ OpEntryPoint Vertex %5 "m2" %100 %101 %102
+ OpExecutionMode %4 OriginUpperLeft
+ OpSource ESSL 310
+ %2 = OpTypeVoid
+ %3 = OpTypeFunction %2
+ %6 = OpTypeFloat 32
+ %7 = OpTypeInt 32 1
+ %8 = OpTypeVector %6 2
+ %9 = OpTypePointer Function %6
+ %10 = OpTypePointer Private %6
+ %20 = OpTypePointer Uniform %6
+ %11 = OpTypePointer Function %7
+ %12 = OpTypePointer Private %7
+ %13 = OpTypePointer Private %8
+ %14 = OpVariable %10 Private
+ %15 = OpVariable %20 Uniform
+ %16 = OpConstant %7 1
+ %17 = OpTypePointer Private %10
+ %18 = OpTypeBool
+ %19 = OpTypePointer Private %18
+ %21 = OpConstantTrue %18
+ %100 = OpVariable %12 Private
+ %101 = OpVariable %12 Private %16
+ %102 = OpVariable %19 Private %21
+ %4 = OpFunction %2 None %3
+ %30 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ %5 = OpFunction %2 None %3
+ %31 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ )";
+ ASSERT_TRUE(IsEqual(env, after_transformation, context.get()));
+}
+
} // namespace
} // namespace fuzz
} // namespace spvtools
