google / amber

Amber is a multi-API shader test framework
Apache License 2.0
188 stars 63 forks source link

google/vulkan-performance-layers requires a call to vkDeviceWaitIdle … #1028

Closed phoenixxxx closed 10 months ago

phoenixxxx commented 1 year ago

…or vkQueueWaitIdle. Since we want to be able to use that layer in conjunction with Amber we need to somehow communicate that the Amber script has completed.

google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

s-perron commented 1 year ago

@phoenixxxx This failed most of the builds. One of the error messages is:

FAILED: src/CMakeFiles/amber_unittests.dir/vulkan/pipeline_test.cc.o
/usr/bin/g++  -DAMBER_CTS_VULKAN_HEADER=0 -DAMBER_ENABLE_CLSPV=1 -DAMBER_ENABLE_DXC=0 -DAMBER_ENABLE_LODEPNG=1 -DAMBER_ENABLE_RTTI=0 -DAMBER_ENABLE_SHADERC=1 -DAMBER_ENABLE_SPIRV_TOOLS=1 -DAMBER_ENGINE_DAWN=0 -DAMBER_ENGINE_VULKAN=1 -I../include -I../ -I../third_party/spirv-tools/include -I../third_party/vulkan-headers/include -I. -I../third_party/clspv/include -Ithird_party/clspv/include -I../third_party/shaderc/libshaderc/include -I../third_party/glslang/SPIRV/.. -Iinclude -I../third_party/glslang/SPIRV/../External -isystem ../third_party/googletest/googlemock/include -isystem ../third_party/googletest/googlemock -isystem ../third_party/googletest/googletest/include -isystem ../third_party/googletest/googletest -O2 -g -DNDEBUG -fPIE   -Wno-global-constructors -Wno-weak-vtables -fno-exceptions -fvisibility=hidden -Wall -Wextra -Wno-cast-function-type-strict -Wno-padded -Wno-switch-enum -Wno-unknown-pragmas -pedantic-errors -Werror -fno-rtti -std=gnu++17 -MD -MT src/CMakeFiles/amber_unittests.dir/vulkan/pipeline_test.cc.o -MF src/CMakeFiles/amber_unittests.dir/vulkan/pipeline_test.cc.o.d -o src/CMakeFiles/amber_unittests.dir/vulkan/pipeline_test.cc.o -c ../src/vulkan/pipeline_test.cc
../src/vulkan/pipeline_test.cc: In member function 'virtual void amber::vulkan::VulkanPipelineTest_AddBufferDescriptorAddPushConstant_Test::TestBody()':
../src/vulkan/pipeline_test.cc:29:52: error: no matching function for call to 'amber::vulkan::ComputePipeline::ComputePipeline(std::nullptr_t, int, std::vector<VkPipelineShaderStageCreateInfo>&)'
   29 |   ComputePipeline pipeline(nullptr, 0, create_infos);
      |                                                    ^
In file included from ../src/vulkan/pipeline_test.cc:19:
../src/vulkan/compute_pipeline.h:30:3: note: candidate: 'amber::vulkan::ComputePipeline::ComputePipeline(amber::vulkan::Device*, uint32_t, bool, const std::vector<VkPipelineShaderStageCreateInfo>&)'
   30 |   ComputePipeline(
      |   ^~~~~~~~~~~~~~~
../src/vulkan/compute_pipeline.h:30:3: note:   candidate expects 4 arguments, 3 provided
../src/vulkan/pipeline_test.cc: In member function 'virtual void amber::vulkan::VulkanPipelineTest_AddBufferDescriptorAddBufferTwice_Test::TestBody()':
../src/vulkan/pipeline_test.cc:41:52: error: no matching function for call to 'amber::vulkan::ComputePipeline::ComputePipeline(std::nullptr_t, int, std::vector<VkPipelineShaderStageCreateInfo>&)'
   41 |   ComputePipeline pipeline(nullptr, 0, create_infos);
      |                                                    ^
In file included from ../src/vulkan/pipeline_test.cc:19:
../src/vulkan/compute_pipeline.h:30:3: note: candidate: 'amber::vulkan::ComputePipeline::ComputePipeline(amber::vulkan::Device*, uint32_t, bool, const std::vector<VkPipelineShaderStageCreateInfo>&)'
   30 |   ComputePipeline(
      |   ^~~~~~~~~~~~~~~
../src/vulkan/compute_pipeline.h:30:3: note:   candidate expects 4 arguments, 3 provided

Please take a look.

dneto0 commented 10 months ago

Hi. @phoenixxxx we still need you to sign the CLA. Also, please rebase your changes on the main branch.

s-perron commented 10 months ago

Hi. @phoenixxxx we still need you to sign the CLA. Also, please rebase your changes on the main branch.

I believe the email address you use needs to be your @google.com address for the CLA to be automatically signed.

https://github.com/google/amber/pull/1028/checks?check_run_id=14485153716

phoenixxxx commented 10 months ago

Hi,

I had added my @google email address to my git account and made a different commit with that email, which was accepted and merged.

Thanks, -Serge

On Wed, Aug 30, 2023 at 8:30 AM Steven Perron @.***> wrote:

Hi. @phoenixxxx https://github.com/phoenixxxx we still need you to sign the CLA. Also, please rebase your changes on the main branch.

I believe the email address you use needs to be your @google address for the CLA to be automatically signed.

https://github.com/google/amber/pull/1028/checks?check_run_id=14485153716

— Reply to this email directly, view it on GitHub https://github.com/google/amber/pull/1028#issuecomment-1699399582, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABV4TBRDDGKNQO6PZDNXPUTXX5MB3ANCNFSM6AAAAAAZIGQCLM . You are receiving this because you were mentioned.Message ID: @.***>

phoenixxxx commented 10 months ago

I think maybe this PR should be closed?