Postpone .amber file parsing

All .amber files were parsed during test indexing, so they were parsed
even if none of the tests were executed. This wasn't a big performance
hit yet, but we're likely to increase the use of amber.

Due to some chicken-and-egg issues part of the capability check was
moved to the start of iterate, but since that throws InternalError if
the check fails this shouldn't be a performance issue.

The tests themselves have not changed, just the order of when to parse
the file.

Affects: None

Components: Vulkan, Framework

VK-GL-CTS issue: 899

Change-Id: Iacdfd9a61d439e3f5dec952b2426386a2eb3451c

diff --git a/external/vulkancts/modules/vulkan/amber/vktAmberTestCase.cpp b/external/vulkancts/modules/vulkan/amber/vktAmberTestCase.cpp
index 5586bb7..eafc53a 100644 (file)
--- a/external/vulkancts/modules/vulkan/amber/vktAmberTestCase.cpp
+++ b/external/vulkancts/modules/vulkan/amber/vktAmberTestCase.cpp
@@ -43,11 +43,13 @@ namespace vkt
 namespace cts_amber
 {
 
-AmberTestCase::AmberTestCase (tcu::TestContext& testCtx,
-                                                         const char*           name,
-                                                         const char*           description)
+AmberTestCase::AmberTestCase (tcu::TestContext&                testCtx,
+                                                         const char*                   name,
+                                                         const char*                   description,
+                                                         const std::string&    readFilename)
        : TestCase(testCtx, name, description),
-         m_recipe(DE_NULL)
+         m_recipe(DE_NULL),
+         m_readFilename(readFilename)
 {
 }
 
@@ -95,6 +97,16 @@ static bool isFeatureSupported(const vkt::Context& ctx, const std::string& featu
        TCU_THROW(InternalError, message.c_str());
 }
 
+void AmberTestCase::delayedInit(void)
+{
+       // Make sure the input can be parsed before we use it.
+       if (!parse(m_readFilename))
+       {
+               std::string message = "Failed to parse Amber file: " + m_readFilename;
+               TCU_THROW(InternalError, message.c_str());
+       }
+}
+
 void AmberTestCase::checkSupport(Context& ctx) const
 {
        // Check for instance and device extensions as declared by the test code.
@@ -146,36 +158,10 @@ void AmberTestCase::checkSupport(Context& ctx) const
                        TCU_THROW(NotSupportedError, message.c_str());
                }
        }
-
-       // Check for extensions as declared by the Amber script itself.  Throw an internal
-       // error if that's more demanding.
-       amber::Amber                    am;
-       amber::Options                  amber_options;
-       amber_options.engine    = amber::kEngineTypeVulkan;
-       amber_options.config    = createEngineConfig(ctx);
-       amber_options.delegate  = DE_NULL;
-
-       amber::Result r = am.AreAllRequirementsSupported(m_recipe, &amber_options);
-       if (!r.IsSuccess())
-       {
-               // dEQP does not to rely on external code to determine whether
-               // a test is supported.  So throw an internal error here instead
-               // of a NotSupportedError.  If an Amber test is not supported, then
-               // you must override this method and throw a NotSupported exception
-               // before reach here.
-               TCU_THROW(InternalError, r.Error().c_str());
-       }
-
-       delete amber_options.config;
 }
 
-bool AmberTestCase::parse(const char* category, const std::string& filename)
+bool AmberTestCase::parse(const std::string& readFilename)
 {
-       std::string readFilename("vulkan/amber/");
-       readFilename.append(category);
-       readFilename.append("/");
-       readFilename.append(filename);
-
        std::string script = ShaderSourceProvider::getSource(m_testCtx.getArchive(), readFilename.c_str());
        if (script.empty())
                return false;
@@ -265,7 +251,28 @@ void AmberTestCase::initPrograms(vk::SourceCollections& programCollection) const
 
 tcu::TestStatus AmberTestInstance::iterate (void)
 {
-       amber::ShaderMap shaderMap;
+       amber::Amber            am;
+       amber::Options          amber_options;
+       amber::ShaderMap        shaderMap;
+       amber::Result           r;
+
+       amber_options.engine                    = amber::kEngineTypeVulkan;
+       amber_options.config                    = createEngineConfig(m_context);
+       amber_options.delegate                  = DE_NULL;
+       amber_options.execution_type    = amber::ExecutionType::kExecute;
+
+       // Check for extensions as declared by the Amber script itself.  Throw an internal
+       // error if that's more demanding.
+       r = am.AreAllRequirementsSupported(m_recipe, &amber_options);
+       if (!r.IsSuccess())
+       {
+               // dEQP does not to rely on external code to determine whether
+               // a test is supported.  So throw an internal error here instead
+               // of a NotSupportedError.  If an Amber test is not supported, then
+               // you must override this method and throw a NotSupported exception
+               // before reach here.
+               TCU_THROW(InternalError, r.Error().c_str());
+       }
 
        std::vector<amber::ShaderInfo> shaders = m_recipe->GetShaderInfo();
        for (size_t i = 0; i < shaders.size(); ++i)
@@ -286,14 +293,7 @@ tcu::TestStatus AmberTestInstance::iterate (void)
                shaderMap[shader.shader_name] = data;
        }
 
-       amber::Amber                                            am;
-       amber::Options                                          amber_options;
-       amber_options.engine                            = amber::kEngineTypeVulkan;
-       amber_options.config                            = createEngineConfig(m_context);
-       amber_options.delegate                          = DE_NULL;
-       amber_options.execution_type            = amber::ExecutionType::kExecute;
-
-       amber::Result r = am.ExecuteWithShaderData(m_recipe, &amber_options, shaderMap);
+       r = am.ExecuteWithShaderData(m_recipe, &amber_options, shaderMap);
        if (!r.IsSuccess()) {
                m_context.getTestContext().getLog()
                        << tcu::TestLog::Message

diff --git a/external/vulkancts/modules/vulkan/amber/vktAmberTestCase.hpp b/external/vulkancts/modules/vulkan/amber/vktAmberTestCase.hpp
index 24111a4..8a04c9c 100644 (file)
--- a/external/vulkancts/modules/vulkan/amber/vktAmberTestCase.hpp
+++ b/external/vulkancts/modules/vulkan/amber/vktAmberTestCase.hpp
@@ -58,7 +58,8 @@ class AmberTestCase : public TestCase
 public:
        AmberTestCase   (tcu::TestContext&      testCtx,
                                         const char*            name,
-                                        const char*            description);
+                                        const char*            description,
+                                        const std::string&     readFilename);
 
        virtual ~AmberTestCase (void);
 
@@ -73,13 +74,13 @@ public:
        //  - Otherwise, we do a secondary sanity check depending on code inside
        //    Amber itself: if the Amber test says it is not supported, then
        //    throw an internal error exception.
-       virtual void checkSupport(Context& ctx) const; // override
+       virtual void checkSupport (Context& ctx) const; // override
 
-       bool parse(const char* category, const std::string& filename);
-       void initPrograms(vk::SourceCollections& programCollection) const;
        // If the test case uses SPIR-V Assembly, use these build options.
        // Otherwise, defaults to target Vulkan 1.0, SPIR-V 1.0.
        void setSpirVAsmBuildOptions(const vk::SpirVAsmBuildOptions& asm_options);
+       virtual void delayedInit (void);
+       virtual void initPrograms (vk::SourceCollections& programCollection) const;
 
        // Add a required instance extension, device extension, or feature bit.
        // A feature bit is represented by a string of form "<structure>.<feature>", where
@@ -89,9 +90,13 @@ public:
        void addRequirement(const std::string& requirement);
 
 private:
+       bool parse (const std::string& readFilename);
+
        amber::Recipe* m_recipe;
        vk::SpirVAsmBuildOptions m_asm_options;
 
+       std::string m_readFilename;
+
        // Instance and device extensions required by the test.
        // We don't differentiate between the two:  We consider the requirement
        // satisfied if the string is registered as either an instance or device

diff --git a/external/vulkancts/modules/vulkan/amber/vktAmberTestCaseUtil.cpp b/external/vulkancts/modules/vulkan/amber/vktAmberTestCaseUtil.cpp
index 3b20b30..d585441 100644 (file)
--- a/external/vulkancts/modules/vulkan/amber/vktAmberTestCaseUtil.cpp
+++ b/external/vulkancts/modules/vulkan/amber/vktAmberTestCaseUtil.cpp
@@ -34,24 +34,18 @@ AmberTestCase* createAmberTestCase (tcu::TestContext&                               testCtx,
                                                                        const std::string&                              filename,
                                                                        const std::vector<std::string>  requirements)
 {
-       AmberTestCase *testCase = new AmberTestCase(testCtx, name, description);
+       // shader_test files are saved in <path>/external/vulkancts/data/vulkan/amber/<categoryname>/
+       std::string readFilename("vulkan/amber/");
+       readFilename.append(category);
+       readFilename.append("/");
+       readFilename.append(filename);
+
+       AmberTestCase *testCase = new AmberTestCase(testCtx, name, description, readFilename);
 
        for (auto req : requirements)
                testCase->addRequirement(req);
 
-       // shader_test files are saved in <path>/external/vulkancts/data/vulkan/amber/<categoryname>/
-       // Make sure the input can be parsed before we use it.
-       if (testCase->parse(category, filename))
-               return testCase;
-       else
-       {
-               const std::string msg = "Failed to parse Amber file: " + filename;
-
-               delete testCase;
-               TCU_THROW(InternalError, msg.c_str());
-       }
-
-       return DE_NULL;
+       return testCase;
 }
 
 } // cts_amber