Skip to content

Unit tests and benchmark for subgroup2 and workgroup2 stuff #192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 39 commits into
base: master
Choose a base branch
from

Conversation

keptsecret
Copy link
Contributor

No description provided.

@keptsecret
Copy link
Contributor Author

Intended to replace #190 due to rearranging/renaming of examples

@keptsecret keptsecret mentioned this pull request May 7, 2025
Comment on lines +94 to +95
add_subdirectory(74_Arithmetic2Bench EXCLUDE_FROM_ALL)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take nr 29 for it

Comment on lines +4 to +5
#include "nbl/builtin/hlsl/subgroup/basic.hlsl"
#include "nbl/builtin/hlsl/subgroup/arithmetic_portability.hlsl"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't include non subgroup2 headers

Comment on lines +19 to +22
// unfortunately DXC chokes on descriptors as static members
// https://github.com/microsoft/DirectXShaderCompiler/issues/5940
[[vk::binding(0, 0)]] StructuredBuffer<type_t> inputValue;
[[vk::binding(1, 0)]] RWByteAddressBuffer output[8];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make the test use BDA so its simpler (no descriptor sets, just BDA sent via push constants)

Comment on lines -141 to -173
const auto spirv_isa_cache_path = localOutputCWD/"spirv_isa_cache.bin";
// enclose to make sure file goes out of scope and we can reopen it
{
smart_refctd_ptr<const IFile> spirv_isa_cache_input;
// try to load SPIR-V to ISA cache
{
ISystem::future_t<smart_refctd_ptr<IFile>> fileCreate;
m_system->createFile(fileCreate,spirv_isa_cache_path,IFile::ECF_READ|IFile::ECF_MAPPABLE|IFile::ECF_COHERENT);
if (auto lock=fileCreate.acquire())
spirv_isa_cache_input = *lock;
}
// create the cache
{
std::span<const uint8_t> spirv_isa_cache_data = {};
if (spirv_isa_cache_input)
spirv_isa_cache_data = {reinterpret_cast<const uint8_t*>(spirv_isa_cache_input->getMappedPointer()),spirv_isa_cache_input->getSize()};
else
m_logger->log("Failed to load SPIR-V 2 ISA cache!",ILogger::ELL_PERFORMANCE);
// Normally we'd deserialize a `ICPUPipelineCache` properly and pass that instead
m_spirv_isa_cache = m_device->createPipelineCache(spirv_isa_cache_data);
}
}
{
// TODO: rename `deleteDirectory` to just `delete`? and a `IFile::setSize()` ?
m_system->deleteDirectory(spirv_isa_cache_path);
ISystem::future_t<smart_refctd_ptr<IFile>> fileCreate;
m_system->createFile(fileCreate,spirv_isa_cache_path,IFile::ECF_WRITE);
// I can be relatively sure I'll succeed to acquire the future, the pointer to created file might be null though.
m_spirv_isa_cache_output=*fileCreate.acquire();
if (!m_spirv_isa_cache_output)
logFail("Failed to Create SPIR-V to ISA cache file.");
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a very useful cache, please restore

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was also a test of the caching mechanism itself

@@ -457,6 +477,9 @@ class ArithmeticUnitTestApp final : public application_templates::BasicMultiQueu
smart_refctd_ptr<ICPUBuffer> resultsBuffer;

uint32_t totalFailCount = 0;

constexpr static inline std::array<uint32_t, 4> WorkgroupSizes = { 32, 256, 512, 1024 };

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need all powers of two from 4 to 2048

Comment on lines +36 to +37
template<template<class> class binop, typename T, uint32_t N>
static void subtest(NBL_CONST_REF_ARG(type_t) sourceVal)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grab a binop thats already instantiated (use vector_traits on type_t to get your scalar type), and get the T out of it

N you can also get with vector traits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants