Add capability attribute

[XAMPPRocky]

This PR adds the #[spirv(capabilities)] attribute, which accepts a list of capabilities on an entrypoint function and emits those capabilities.

To Do

• Documentation
• Ensure you can only use #[spirv(capabilities)] with entry point functions.

[dragostis]

Just out of curiosity, could capabilities be detected at compile time and would that be desirable?

[XAMPPRocky]

Just out of curiosity, could capabilities be detected at compile time and would that be desirable?

@dragostis In theory, I believe so, I believe you probably could visit each instruction, see what capabilities it requires, and construct a set of a capabilities to emit at the end.

However whether that's desirable is another part, because as I understand, these capabilities affect platform availability, meaning that some platforms don't have certain capabilities. So unintentionally enabling them can be a frustrating experience as your code will compile successfully to SPIR-V, but will fail when that SPIR-V is compiled on the target platform.

[khyperia]

@dragostis In theory, I believe so, I believe you probably could visit each instruction, see what capabilities it requires, and construct a set of a capabilities to emit at the end.

FWIW we kind of do that right now, and we're moving away from that.

rust-gpu/crates/rustc_codegen_spirv/src/linker/capability_computation.rs

Lines 28 to 30 in 908487a

	let mut set = HashSet::new();
	for inst in module.all_inst_iter() {
	set.extend(inst.class.capabilities);

(that file should also go away with this PR, along with this

rust-gpu/crates/rustc_codegen_spirv/src/builder_spirv.rs

Lines 223 to 226 in 908487a

	builder.capability(Capability::Int8);
	builder.capability(Capability::Int16);
	builder.capability(Capability::Int64);
	builder.capability(Capability::Float64);

)

[XAMPPRocky]

@khyperia Removing capability_computation is fine, however removing the defaults in builder_spirv.rs prevents glam from being compiled.

  error: f64 without OpCapability Float64
     --> /Users/erin.power/.rustup/toolchains/nightly-2021-03-21-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/num/f64.rs:640:5
      |
  640 |     pub fn recip(self) -> f64 {
      |     ^^^^^^^^^^^^^^^^^^^^^^^^^

  error: aborting due to previous error

  error: could not compile `glam`

[khyperia]

@khyperia Removing capability_computation is fine, however removing the defaults in builder_spirv.rs prevents glam from being compiled.

Yes, those systems in the compiler will need to be updated to the new capability computation system, in this case, my comment

perhaps functions using u64 are automatically declared with needs_capability = Int64 implicitly, to avoid needing to modify libraries

in #42.

(Don't remove capability_computation without removing the defaults in builder_spirv.rs, the two complement each other, and unexpected capabilities will arise if you remove one without the other)

[XAMPPRocky]

(Don't remove capability_computation without removing the defaults in builder_spirv.rs, the two complement each other, and unexpected capabilities will arise if you remove one without the other)

Okay, well I think we should make removing those a separate PR, as they aren't incompatible with the new attribute, and it keeps both PRs smaller and easier to test.

[khyperia]

Okay, well I think we should make removing those a separate PR, as they aren't incompatible with the new attribute, and it keeps both PRs smaller and easier to test.

Sorry, I guess I was confused by the scope of this PR

[XAMPPRocky]

Sorry, I guess I was confused by the scope of this PR

No worries, I don't think there was a explicit scope when I opened, and this discussion helped clarified it. 😄

[khyperia]

I think multiple.rs and invalid-target.rs need to be updated with this. not_entry_point.rs can probably also be folded into invalid-target.rs?

[khyperia]

This should be cached and put in a HashMap, similar to how const BUILTINS/Symbols::attributes is done - interning is relatively expensive, especially in a O(n) find loop. The array should probably also be defined alongside the other large chunks of data, to consolidate them into something more manageable.

[khyperia]

Going with #555 instead of attributes, so closing this

[khyperia]

rip, race condition on closing it, hit the button literally the moment github flipped it to "reopen", haha