Skip to content

Fix support of float atomics in threadgroup memory.#1

Open
duburcqa wants to merge 1 commit intomainfrom
duburcqa/fix_metal_atomics_shared_mem
Open

Fix support of float atomics in threadgroup memory.#1
duburcqa wants to merge 1 commit intomainfrom
duburcqa/fix_metal_atomics_shared_mem

Conversation

@duburcqa
Copy link
Copy Markdown

@duburcqa duburcqa commented Mar 29, 2026

Cross-referencing upstream PR: KhronosGroup#2611

@hughperkins
Copy link
Copy Markdown
Collaborator

This main branch is not used. The correct main is at https://github.com/KhronosGroup/SPIRV-Cross

What we can do is to create an integration branch, that contains both our fixes. e.g. gs_integration, perhaps?

Or I suppose we could treat main as an integration branch, but that seems unintuitive to me? 🤔 Not sure 🤔

@hughperkins
Copy link
Copy Markdown
Collaborator

Change looks reasonable to me. Please could you post a self-declaration. Choose your preferred self-declaration:

"I have read every line added in this PR, and reviewed the lines. I take responsibilty for the lines added and removed in this PR, and won't blame any issues on Opus, or any other AI."

"I wrote this code myself. I take responsibilty for the lines added and removed in this PR, and won't blame any issues on Opus or any other AI."

(or propose another that aligns with my intentions here, but avoids raising any concerns you may have over my own choice of wording)

@hughperkins
Copy link
Copy Markdown
Collaborator

Also, please could you submit a PR to upstream, and submit a link here.

@duburcqa
Copy link
Copy Markdown
Author

I was assisted by Claude Opus to write this PR. I have read every line added in this PR, and reviewed the lines. I take full responsibility for the lines added and removed in this PR. I won't blame any issue on Claude Opus.

@duburcqa duburcqa changed the title Fix Support of float atomics in threadgroup memory. Fix support of float atomics in threadgroup memory. Mar 29, 2026
@hughperkins
Copy link
Copy Markdown
Collaborator

Could we add a unit test for this issue please, that fails without this change, and passes with this change.

@hughperkins
Copy link
Copy Markdown
Collaborator

Note: revoked requiemetn for unit tset for this PR, because it 'plays poorly' with the requirement to hvae read and understood all lines of the code. Baby steps. We will ensure first that hte tests on Quadrants are passing though.

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