Skip to content

[Test] Gate extension-requiring tests on actual device capabilities instead of a static arch table.#803

Closed
duburcqa wants to merge 1 commit into
mainfrom
duburcqa/resolve_require_extensions_per_device
Closed

[Test] Gate extension-requiring tests on actual device capabilities instead of a static arch table.#803
duburcqa wants to merge 1 commit into
mainfrom
duburcqa/resolve_require_extensions_per_device

Conversation

@duburcqa

Copy link
Copy Markdown
Contributor

Issue: #

Whether the adstack and data64 extensions are actually usable on the SPIR-V archs (vulkan, metal) is a
property of the initialized device, not of the arch alone: the on-device adstack size interpreter reads through raw
64-bit buffer addresses, so it needs spirv_has_physical_storage_buffer + spirv_has_int64 (the launch gate in
runtime/gfx/adstack_sizer_launch.cpp), and data64 needs spirv_has_int64 + spirv_has_float64. The static
arch-level extension table cannot advertise this, so @test_utils.test(require=...) used to resolve support at
collection time against that table -- which is both too coarse (no device exists yet) and wrong on the SPIR-V archs.

This PR:

  • makes is_extension_enabled consult the initialized device's capabilities for adstack / data64 on the SPIR-V
    archs, superseding the static table where the device is the authority;
  • moves the test harness's require= resolution from collection time into the wanted_arch fixture: the required
    extensions ride the parametrization and are checked against the actual device after qd.init, skipping with a
    clear reason when the device does not support one.

No behavior change on CPU / CUDA / AMDGPU, where the arch table already matches the device.

Brief Summary

copilot:summary

Walkthrough

copilot:walkthrough

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d382cd7918

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +855 to +856
return bool(caps.get(_qd_core.DeviceCapability.spirv_has_physical_storage_buffer)) and bool(
caps.get(_qd_core.DeviceCapability.spirv_has_int64)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Gate adstack on all SPIR-V sizer capabilities

On a Vulkan device that advertises physical-storage buffers and int64 but lacks int8 or int16, this returns True for qd.extension.adstack, so the updated test fixture will run require=qd.extension.adstack cases instead of skipping them. The actual SPIR-V adstack sizer still hard-errors unless spirv_has_physical_storage_buffer, spirv_has_int64, spirv_has_int8, and spirv_has_int16 are all present (runtime/gfx/adstack_sizer_launch.cpp:332-337), so this check needs to match that runtime gate.

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

@duburcqa duburcqa closed this Jul 21, 2026
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.

1 participant