Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Add is_helper_invocation, and cleanup arch & asm functions#612

Merged
XAMPPRocky merged 1 commit into
mainfrom
cleanup
May 4, 2021
Merged

Add is_helper_invocation, and cleanup arch & asm functions#612
XAMPPRocky merged 1 commit into
mainfrom
cleanup

Conversation

@XAMPPRocky

Copy link
Copy Markdown
Member

Cleaning up and removing all the asm uses of OpCapability, and also added is_helper_invocation, also made demote_to_helper_invocation unsafe, as using it has side effects that make variables undefined.

@XAMPPRocky XAMPPRocky requested review from eddyb and khyperia May 4, 2021 07:54
@XAMPPRocky XAMPPRocky force-pushed the cleanup branch 2 times, most recently from 41809a4 to eda58b4 Compare May 4, 2021 07:55
/// non-uniform) and does not terminate the block.
///
/// - **Required Capabilities** `DemoteToHelperInvocationEXT`
/// - **Required Extensions** `SPV_EXT_demote_to_helper_invocation`

@khyperia khyperia May 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Turning these into #[cfg] on the method would be nice and is something we should do eventually, but not required right now. (Same thing on derivatives, etc.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, and I think we'd want to do more than #[cfg], as when it's cfg'ed out, the error will just be "demote_to_helper_invocation not found", when ideally the error message would tell you, you need those capabilities and extensions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... as in, doing a custom cfg attribute? we already decided against that in our discussions though, I'm confused

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure how it would be exactly, but I meant more like a macro which just expands to two versions of the function containing the cfg and a better message similar to gpu_only.

Comment thread crates/spirv-std/src/arch.rs
@XAMPPRocky XAMPPRocky enabled auto-merge (squash) May 4, 2021 10:47
@XAMPPRocky XAMPPRocky merged commit 9c19414 into main May 4, 2021
@XAMPPRocky XAMPPRocky deleted the cleanup branch May 4, 2021 10:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

2 participants