Skip to content

[Comgr] Add atomic noise flags to SPIR-V flag extraction#2183

Open
lamb-j wants to merge 1 commit intoamd-mainfrom
comgr/spirv-atomic-flags
Open

[Comgr] Add atomic noise flags to SPIR-V flag extraction#2183
lamb-j wants to merge 1 commit intoamd-mainfrom
comgr/spirv-atomic-flags

Conversation

@lamb-j
Copy link
Copy Markdown
Collaborator

@lamb-j lamb-j commented Apr 13, 2026

Summary

  • Add -fatomic-fine-grained-memory, -fatomic-ignore-denormal-mode, and -fatomic-remote-memory to Comgr's ValidSpirvFlags allowlist so they are preserved through the SPIR-V → relocatable compilation pipeline
  • Update the spirv-to-reloc.hip lit test to cover extraction of these flags

Add -fatomic-fine-grained-memory, -fatomic-ignore-denormal-mode, and
-fatomic-remote-memory to the ValidSpirvFlags allowlist so they are
preserved through the SPIR-V compilation pipeline.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@z1-cciauto
Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown

@chinmaydd chinmaydd left a comment

Choose a reason for hiding this comment

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

I wonder if there's a better way to maintain this list of "allowed" flags...

@z1-cciauto
Copy link
Copy Markdown
Collaborator

@chinmaydd chinmaydd added the comgr Related to Code Object Manager label Apr 13, 2026
@AlexVlx
Copy link
Copy Markdown

AlexVlx commented Apr 13, 2026

We probably need to include the negations i.e. the -fno forms as well. Also, for legacy purposes, might be nice to include -munsafe-fp-atomics and -mnounsafe-fp-atomics

@AlexVlx
Copy link
Copy Markdown

AlexVlx commented Apr 13, 2026

I wonder if there's a better way to maintain this list of "allowed" flags...

The long-term plan is / was to start with all known / allowed -cc1 flags, and then mask out the few that are an issue.

@chinmaydd
Copy link
Copy Markdown

The long-term plan is / was to start with all known / allowed -cc1 flags, and then mask out the few that are an issue

That makes sense, thanks for the clarification.

@lamb-j
Copy link
Copy Markdown
Collaborator Author

lamb-j commented Apr 13, 2026

We probably need to include the negations i.e. the -fno forms as well. Also, for legacy purposes, might be nice to include -munsafe-fp-atomics and -mnounsafe-fp-atomics

Doesn't look like the -fno forms or aliases will make it to the CC1 invocation (so not in @llvm.cmdline). They get converted to one of the above, or omitted (as they're default off)

Do we think the defaults for these will ever change? If so we could future-proof by adding the -fno versions I guess

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comgr Related to Code Object Manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants