Skip to content

Commit 1cda495

Browse files
dneto0dneto0
authored
Use less stack space when validating Vulkan builtins (KhronosGroup#4019)
Don't pass a constructed string as an argument to ValidateNotCalledwithExecutionModel. That method is captured via std::bind and it ends up using lots of stack space. Instead, pass in: - a Vulkan validation unique ID as an integer instead, (with -1 meaning no VUID), and - the const char* for the explanator text.
1 parent a61d07a commit 1cda495

File tree

1 file changed

+49
-58
lines changed

1 file changed

+49
-58
lines changed

‎source/val/validate_builtins.cpp

Lines changed: 49 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -403,14 +403,15 @@ class BuiltInsValidator {
403403
// Validates that |built_in_inst| is not (even indirectly) referenced from
404404
// within a function which can be called with |execution_model|.
405405
//
406+
// |vuid| - Vulkan ID for the error, or a negative value if none.
406407
// |comment| - text explaining why the restriction was imposed.
407408
// |decoration| - BuiltIn decoration which causes the restriction.
408409
// |referenced_inst| - instruction which is dependent on |built_in_inst| and
409410
// defines the id which was referenced.
410411
// |referenced_from_inst| - instruction which references id defined by
411412
// |referenced_inst| from within a function.
412413
spv_result_t ValidateNotCalledWithExecutionModel(
413-
std::string comment, SpvExecutionModel execution_model,
414+
int vuid, const char* comment, SpvExecutionModel execution_model,
414415
const Decoration& decoration, const Instruction& built_in_inst,
415416
const Instruction& referenced_inst,
416417
const Instruction& referenced_from_inst);
@@ -909,7 +910,7 @@ spv_result_t BuiltInsValidator::ValidateF32ArrHelper(
909910
}
910911

911912
spv_result_t BuiltInsValidator::ValidateNotCalledWithExecutionModel(
912-
std::string comment, SpvExecutionModel execution_model,
913+
int vuid, const char* comment, SpvExecutionModel execution_model,
913914
const Decoration& decoration, const Instruction& built_in_inst,
914915
const Instruction& referenced_inst,
915916
const Instruction& referenced_from_inst) {
@@ -920,7 +921,8 @@ spv_result_t BuiltInsValidator::ValidateNotCalledWithExecutionModel(
920921
const char* built_in_str = _.grammar().lookupOperandName(
921922
SPV_OPERAND_TYPE_BUILT_IN, decoration.params()[0]);
922923
return _.diag(SPV_ERROR_INVALID_DATA, &referenced_from_inst)
923-
<< comment << " " << GetIdDesc(referenced_inst) << " depends on "
924+
<< (vuid < 0 ? std::string("") : _.VkErrorID(vuid)) << comment
925+
<< " " << GetIdDesc(referenced_inst) << " depends on "
924926
<< GetIdDesc(built_in_inst) << " which is decorated with BuiltIn "
925927
<< built_in_str << "."
926928
<< " Id <" << referenced_inst.id() << "> is later referenced by "
@@ -932,7 +934,7 @@ spv_result_t BuiltInsValidator::ValidateNotCalledWithExecutionModel(
932934
// Propagate this rule to all dependant ids in the global scope.
933935
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(
934936
std::bind(&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
935-
comment, execution_model, decoration, built_in_inst,
937+
vuid, comment, execution_model, decoration, built_in_inst,
936938
referenced_from_inst, std::placeholders::_1));
937939
}
938940
return SPV_SUCCESS;
@@ -968,7 +970,7 @@ spv_result_t BuiltInsValidator::ValidateClipOrCullDistanceAtReference(
968970
if (storage_class == SpvStorageClassInput) {
969971
assert(function_id_ == 0);
970972
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
971-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
973+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, -1,
972974
"Vulkan spec doesn't allow BuiltIn ClipDistance/CullDistance to be "
973975
"used for variables with Input storage class if execution model is "
974976
"Vertex.",
@@ -979,7 +981,7 @@ spv_result_t BuiltInsValidator::ValidateClipOrCullDistanceAtReference(
979981
if (storage_class == SpvStorageClassOutput) {
980982
assert(function_id_ == 0);
981983
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
982-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
984+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, -1,
983985
"Vulkan spec doesn't allow BuiltIn ClipDistance/CullDistance to be "
984986
"used for variables with Output storage class if execution model is "
985987
"Fragment.",
@@ -1619,12 +1621,10 @@ spv_result_t BuiltInsValidator::ValidatePointSizeAtReference(
16191621
if (storage_class == SpvStorageClassInput) {
16201622
assert(function_id_ == 0);
16211623
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
1622-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
1623-
std::string(
1624-
_.VkErrorID(4315) +
1625-
"Vulkan spec doesn't allow BuiltIn PointSize to be used for "
1626-
"variables with Input storage class if execution model is "
1627-
"Vertex."),
1624+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, 4315,
1625+
"Vulkan spec doesn't allow BuiltIn PointSize to be used for "
1626+
"variables with Input storage class if execution model is "
1627+
"Vertex.",
16281628
SpvExecutionModelVertex, decoration, built_in_inst,
16291629
referenced_from_inst, std::placeholders::_1));
16301630
}
@@ -1737,11 +1737,10 @@ spv_result_t BuiltInsValidator::ValidatePositionAtReference(
17371737
if (storage_class == SpvStorageClassInput) {
17381738
assert(function_id_ == 0);
17391739
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
1740-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
1741-
std::string(_.VkErrorID(4320) +
1742-
"Vulkan spec doesn't allow BuiltIn Position to be used "
1743-
"for variables "
1744-
"with Input storage class if execution model is Vertex."),
1740+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, 4320,
1741+
"Vulkan spec doesn't allow BuiltIn Position to be used "
1742+
"for variables "
1743+
"with Input storage class if execution model is Vertex.",
17451744
SpvExecutionModelVertex, decoration, built_in_inst,
17461745
referenced_from_inst, std::placeholders::_1));
17471746
}
@@ -1928,30 +1927,24 @@ spv_result_t BuiltInsValidator::ValidatePrimitiveIdAtReference(
19281927
if (storage_class == SpvStorageClassOutput) {
19291928
assert(function_id_ == 0);
19301929
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
1931-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
1932-
std::string(
1933-
_.VkErrorID(4334) +
1934-
"Vulkan spec doesn't allow BuiltIn PrimitiveId to be used for "
1935-
"variables with Output storage class if execution model is "
1936-
"TessellationControl."),
1930+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, 4334,
1931+
"Vulkan spec doesn't allow BuiltIn PrimitiveId to be used for "
1932+
"variables with Output storage class if execution model is "
1933+
"TessellationControl.",
19371934
SpvExecutionModelTessellationControl, decoration, built_in_inst,
19381935
referenced_from_inst, std::placeholders::_1));
19391936
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
1940-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
1941-
std::string(
1942-
_.VkErrorID(4334) +
1943-
"Vulkan spec doesn't allow BuiltIn PrimitiveId to be used for "
1944-
"variables with Output storage class if execution model is "
1945-
"TessellationEvaluation."),
1937+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, 4334,
1938+
"Vulkan spec doesn't allow BuiltIn PrimitiveId to be used for "
1939+
"variables with Output storage class if execution model is "
1940+
"TessellationEvaluation.",
19461941
SpvExecutionModelTessellationEvaluation, decoration, built_in_inst,
19471942
referenced_from_inst, std::placeholders::_1));
19481943
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
1949-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
1950-
std::string(
1951-
_.VkErrorID(4334) +
1952-
"Vulkan spec doesn't allow BuiltIn PrimitiveId to be used for "
1953-
"variables with Output storage class if execution model is "
1954-
"Fragment."),
1944+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, 4334,
1945+
"Vulkan spec doesn't allow BuiltIn PrimitiveId to be used for "
1946+
"variables with Output storage class if execution model is "
1947+
"Fragment.",
19551948
SpvExecutionModelFragment, decoration, built_in_inst,
19561949
referenced_from_inst, std::placeholders::_1));
19571950
}
@@ -2304,7 +2297,7 @@ spv_result_t BuiltInsValidator::ValidateTessLevelAtReference(
23042297
if (storage_class == SpvStorageClassInput) {
23052298
assert(function_id_ == 0);
23062299
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
2307-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
2300+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, -1,
23082301
"Vulkan spec doesn't allow TessLevelOuter/TessLevelInner to be "
23092302
"used "
23102303
"for variables with Input storage class if execution model is "
@@ -2316,7 +2309,7 @@ spv_result_t BuiltInsValidator::ValidateTessLevelAtReference(
23162309
if (storage_class == SpvStorageClassOutput) {
23172310
assert(function_id_ == 0);
23182311
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
2319-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
2312+
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, -1,
23202313
"Vulkan spec doesn't allow TessLevelOuter/TessLevelInner to be "
23212314
"used "
23222315
"for variables with Output storage class if execution model is "
@@ -2605,32 +2598,30 @@ spv_result_t BuiltInsValidator::ValidateLayerOrViewportIndexAtReference(
26052598
{SpvExecutionModelVertex, SpvExecutionModelTessellationEvaluation,
26062599
SpvExecutionModelGeometry, SpvExecutionModelMeshNV}) {
26072600
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(
2608-
std::bind(
2609-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
2610-
std::string(
2611-
_.VkErrorID((operand == SpvBuiltInLayer) ? 4274 : 4406) +
2612-
"Vulkan spec doesn't allow BuiltIn Layer and "
2613-
"ViewportIndex to be "
2614-
"used for variables with Input storage class if "
2615-
"execution model is Vertex, TessellationEvaluation, "
2616-
"Geometry, or MeshNV."),
2617-
em, decoration, built_in_inst, referenced_from_inst,
2618-
std::placeholders::_1));
2601+
std::bind(&BuiltInsValidator::ValidateNotCalledWithExecutionModel,
2602+
this, ((operand == SpvBuiltInLayer) ? 4274 : 4406),
2603+
"Vulkan spec doesn't allow BuiltIn Layer and "
2604+
"ViewportIndex to be "
2605+
"used for variables with Input storage class if "
2606+
"execution model is Vertex, TessellationEvaluation, "
2607+
"Geometry, or MeshNV.",
2608+
em, decoration, built_in_inst, referenced_from_inst,
2609+
std::placeholders::_1));
26192610
}
26202611
}
26212612

26222613
if (storage_class == SpvStorageClassOutput) {
26232614
assert(function_id_ == 0);
2624-
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
2625-
&BuiltInsValidator::ValidateNotCalledWithExecutionModel, this,
2626-
std::string(_.VkErrorID((operand == SpvBuiltInLayer) ? 4275 : 4407) +
2627-
"Vulkan spec doesn't allow BuiltIn Layer and "
2628-
"ViewportIndex to be "
2629-
"used for variables with Output storage class if "
2630-
"execution model is "
2631-
"Fragment."),
2632-
SpvExecutionModelFragment, decoration, built_in_inst,
2633-
referenced_from_inst, std::placeholders::_1));
2615+
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(
2616+
std::bind(&BuiltInsValidator::ValidateNotCalledWithExecutionModel,
2617+
this, ((operand == SpvBuiltInLayer) ? 4275 : 4407),
2618+
"Vulkan spec doesn't allow BuiltIn Layer and "
2619+
"ViewportIndex to be "
2620+
"used for variables with Output storage class if "
2621+
"execution model is "
2622+
"Fragment.",
2623+
SpvExecutionModelFragment, decoration, built_in_inst,
2624+
referenced_from_inst, std::placeholders::_1));
26342625
}
26352626

26362627
for (const SpvExecutionModel execution_model : execution_models_) {

0 commit comments

Comments
 (0)