Fix obtaining the execution model from the intended entry point
Previously the `executionModel` member was overwritten for each
OpEntryPoint that was being parsed.
This change also refactors constant members that are initialized in the
constructor to not have an in-class initialization value, since those
can be confused to only have that value (like constexpr) or can lead to
forgetting to initialize them in the constructor as intended.
Bug: swiftshader:161
Change-Id: Ie0792bff3e68bbf4bfa2873a189fa6811e508e60
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/57528
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Sean Risser <srisser@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index 4718c3c..4d434ce 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -76,14 +76,16 @@
{
case spv::OpEntryPoint:
{
- executionModel = spv::ExecutionModel(insn.word(1));
- auto id = Function::ID(insn.word(2));
- auto name = insn.string(3);
- auto stage = executionModelToStage(executionModel);
+ spv::ExecutionModel executionModel = spv::ExecutionModel(insn.word(1));
+ Function::ID entryPoint = Function::ID(insn.word(2));
+ const char *name = insn.string(3);
+ VkShaderStageFlagBits stage = executionModelToStage(executionModel);
+
if(stage == pipelineStage && strcmp(name, entryPointName) == 0)
{
- ASSERT_MSG(entryPoint == 0, "Duplicate entry point with name '%s' and stage %d", name, int(stage));
- entryPoint = id;
+ ASSERT_MSG(this->entryPoint == 0, "Duplicate entry point with name '%s' and stage %d", name, int(stage));
+ this->entryPoint = entryPoint;
+ this->executionModel = executionModel;
auto interfaceIdsOffset = 3 + insn.stringSizeInWords(3);
for(uint32_t i = interfaceIdsOffset; i < insn.wordCount(); i++)
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 45bb576..151e69d 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -836,6 +836,11 @@
private:
const uint32_t codeSerialID;
+ const bool robustBufferAccess;
+
+ Function::ID entryPoint;
+ spv::ExecutionModel executionModel = spv::ExecutionModelMax; // Invalid prior to OpEntryPoint parsing.
+
ExecutionModes executionModes = {};
Analysis analysis = {};
Capabilities capabilities = {};
@@ -845,12 +850,8 @@
std::unordered_map<StringID, String> strings;
HandleMap<Extension> extensionsByID;
std::unordered_set<uint32_t> extensionsImported;
- Function::ID entryPoint;
mutable bool imageWriteEmitted = false;
- const bool robustBufferAccess = true;
- spv::ExecutionModel executionModel = spv::ExecutionModelMax; // Invalid prior to OpEntryPoint parsing.
-
// DeclareType creates a Type for the given OpTypeX instruction, storing
// it into the types map. It is called from the analysis pass (constructor).
void DeclareType(InsnIterator insn);
@@ -969,7 +970,7 @@
, multiSampleCount(multiSampleCount)
, executionModel(executionModel)
{
- ASSERT(executionModelToStage(executionModel) != VkShaderStageFlagBits(0)); // Must parse OpEntryPoint before emitting.
+ ASSERT(executionModel != spv::ExecutionModelMax); // Must parse OpEntryPoint before emitting.
}
// Returns the mask describing the active lanes as updated by dynamic
@@ -1063,9 +1064,9 @@
std::unordered_map<Object::ID, Intermediate> intermediates;
std::unordered_map<Object::ID, SIMD::Pointer> pointers;
- const bool robustBufferAccess = true; // Emit robustBufferAccess safe code.
- const unsigned int multiSampleCount = 0;
- const spv::ExecutionModel executionModel = spv::ExecutionModelMax;
+ const bool robustBufferAccess; // Emit robustBufferAccess safe code.
+ const unsigned int multiSampleCount;
+ const spv::ExecutionModel executionModel;
};
// EmitResult is an enumerator of result values from the Emit functions.
