Refactor variable type lookup

Previously the pointee type of OpVariable was determined starting from
the result ID. It just obtains the 'Object' which has the type collected
from the OpVariable instruction itself. So the object lookup can be
skipped by using the latter directly again.

Bug b/129000021

Change-Id: I882d837c4cc1c7a8d0b19de38dc8ee19e622af33
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/27651
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>

diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index 212a0ae..0f6dfdd 100644 (file)
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -199,7 +199,7 @@ namespace sw
                                        break;
 
                                default:
-                                       UNREACHABLE("Unexpected StorageClass"); // See Appendix A of the Vulkan spec.
+                                       UNREACHABLE("Unexpected StorageClass %d", storageClass); // See Appendix A of the Vulkan spec.
                                        break;
                                }
                                break;
@@ -508,13 +508,13 @@ namespace sw
                auto &objectTy = getType(object.type);
                ASSERT(objectTy.storageClass == spv::StorageClassInput || objectTy.storageClass == spv::StorageClassOutput);
 
-               ASSERT(objectTy.definition.opcode() == spv::OpTypePointer);
+               ASSERT(objectTy.opcode() == spv::OpTypePointer);
                auto pointeeTy = getType(objectTy.element);
 
                auto &builtinInterface = (objectTy.storageClass == spv::StorageClassInput) ? inputBuiltins : outputBuiltins;
                auto &userDefinedInterface = (objectTy.storageClass == spv::StorageClassInput) ? inputs : outputs;
 
-               ASSERT(object.definition.opcode() == spv::OpVariable);
+               ASSERT(object.opcode() == spv::OpVariable);
                Object::ID resultId = object.definition.word(2);
 
                if (objectTy.isBuiltInBlock)
@@ -687,7 +687,7 @@ namespace sw
                ApplyDecorationsForId(&d, id);
 
                auto const &obj = getType(id);
-               switch (obj.definition.opcode())
+               switch(obj.opcode())
                {
                case spv::OpTypePointer:
                        return VisitInterfaceInner<F>(obj.definition.word(3), d, f);
@@ -849,7 +849,7 @@ namespace sw
                for (auto i = 0u; i < numIndexes; i++)
                {
                        auto & type = getType(typeId);
-                       switch (type.definition.opcode())
+                       switch(type.opcode())
                        {
                        case spv::OpTypeStruct:
                        {
@@ -881,7 +881,7 @@ namespace sw
                        }
 
                        default:
-                               UNIMPLEMENTED("Unexpected type '%s' in WalkAccessChain", OpcodeName(type.definition.opcode()).c_str());
+                               UNIMPLEMENTED("Unexpected type '%s' in WalkAccessChain", OpcodeName(type.opcode()).c_str());
                        }
                }
 
@@ -895,7 +895,7 @@ namespace sw
                for (auto i = 0u; i < numIndexes; i++)
                {
                        auto & type = getType(typeId);
-                       switch (type.definition.opcode())
+                       switch(type.opcode())
                        {
                        case spv::OpTypeStruct:
                        {
@@ -1070,7 +1070,7 @@ namespace sw
                // but is possible to construct integer constant 0 via OpConstantNull.
                auto insn = getObject(id).definition;
                ASSERT(insn.opcode() == spv::OpConstant);
-               ASSERT(getType(insn.word(1)).definition.opcode() == spv::OpTypeInt);
+               ASSERT(getType(insn.word(1)).opcode() == spv::OpTypeInt);
                return insn.word(3);
        }
 
@@ -1084,14 +1084,14 @@ namespace sw
                        {
                        case spv::OpVariable:
                        {
-                               Object::ID resultId = insn.word(2);
-                               auto &object = getObject(resultId);
-                               auto &objectTy = getType(object.type);
-                               auto &pointeeTy = getType(objectTy.element);
-                               // TODO: what to do about zero-slot objects?
-                               if (pointeeTy.sizeInComponents > 0)
+                               Type::ID resultPointerTypeId = insn.word(1);
+                               auto resultPointerType = getType(resultPointerTypeId);
+                               auto pointeeType = getType(resultPointerType.element);
+
+                               if(pointeeType.sizeInComponents > 0)  // TODO: what to do about zero-slot objects?
                                {
-                                       routine->createLvalue(resultId, pointeeTy.sizeInComponents);
+                                       Object::ID resultId = insn.word(2);
+                                       routine->createLvalue(resultId, pointeeType.sizeInComponents);
                                }
                                break;
                        }
@@ -1466,23 +1466,25 @@ namespace sw
        void SpirvShader::EmitAccessChain(InsnIterator insn, SpirvRoutine *routine) const
        {
                Type::ID typeId = insn.word(1);
-               Object::ID objectId = insn.word(2);
+               Object::ID resultId = insn.word(2);
                Object::ID baseId = insn.word(3);
+               uint32_t numIndexes = insn.wordCount() - 4;
+               const uint32_t *indexes = insn.wordPointer(4);
                auto &type = getType(typeId);
                ASSERT(type.sizeInComponents == 1);
-               ASSERT(getObject(baseId).pointerBase == getObject(objectId).pointerBase);
+               ASSERT(getObject(baseId).pointerBase == getObject(resultId).pointerBase);
 
-               auto &dst = routine->createIntermediate(objectId, type.sizeInComponents);
+               auto &dst = routine->createIntermediate(resultId, type.sizeInComponents);
 
-               if (type.storageClass == spv::StorageClassPushConstant ||
-                       type.storageClass == spv::StorageClassUniform ||
-                       type.storageClass == spv::StorageClassStorageBuffer)
+               if(type.storageClass == spv::StorageClassPushConstant ||
+                  type.storageClass == spv::StorageClassUniform ||
+                  type.storageClass == spv::StorageClassStorageBuffer)
                {
-                       dst.emplace(0, WalkExplicitLayoutAccessChain(baseId, insn.wordCount() - 4, insn.wordPointer(4), routine));
+                       dst.emplace(0, WalkExplicitLayoutAccessChain(baseId, numIndexes, indexes, routine));
                }
                else
                {
-                       dst.emplace(0, WalkAccessChain(baseId, insn.wordCount() - 4, insn.wordPointer(4), routine));
+                       dst.emplace(0, WalkAccessChain(baseId, numIndexes, indexes, routine));
                }
        }
 

diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index ebce2bd..ee0860a 100644 (file)
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -207,6 +207,8 @@ namespace sw
                public:
                        using ID = SpirvID<Type>;
 
+                       spv::Op opcode() const { return definition.opcode(); }
+
                        InsnIterator definition;
                        spv::StorageClass storageClass = static_cast<spv::StorageClass>(-1);
                        uint32_t sizeInComponents = 0;
@@ -221,6 +223,8 @@ namespace sw
                public:
                        using ID = SpirvID<Object>;
 
+                       spv::Op opcode() const { return definition.opcode(); }
+
                        InsnIterator definition;
                        Type::ID type;
                        ID pointerBase;