[Migrated] Remove unneeded Storage Class attributes

[rust-gpu-bot]

Issue automatically imported from old repo: EmbarkStudios/rust-gpu#568
Old labels: t: enhancement,t: good first issue
Originally creatd by XAMPPRocky on 2021-03-31T11:43:53Z

---

With recent changes to how we specify the storage class of a variable, we've managed to eliminate the need for a few of the storage class attributes in cases where we can successfully infer all possible valid uses. Mainly the input, output, private, function, generic, and uniform_constant attributes at the time of this writing are no longer needed and should be removed now that they can't be used for anything useful.

[rust-gpu-bot]

Comment from andrewleverette on 2021-11-24T16:54:47Z

---

I would be happy to work on this. I've found the type and have removed the specified variants. I'm curious how you would want to handle cases where the variants are used. In some cases where they are used for error handling, I think those checks can just be removed. But a case that isn't as clear is handling cases that are dependent on module version like this:

/rustc_codegen_spriv/src/linker/entry_interface.rs

        // Base case: the global itself is an interface-relevant `OpVariable`.
        let interface_relevant_var = inst.class.opcode == Op::Variable && {
            if version > (1, 3) {
                // SPIR-V >= v1.4 includes all OpVariables in the interface.
                true
            } else {
                let storage_class = inst.operands[0].unwrap_storage_class();
                // SPIR-V <= v1.3 only includes Input and Output in the interface.
                storage_class == StorageClass::Input || storage_class == StorageClass::Output
            }
        };

Should the else case just return false? Or is there some other logic that should be added here?

Another case is when a default is used like here:
rustc_codegen_spriv/src/codegen_cx/entry.rs

        // If storage class was not inferred nor specified, compute the default (i.e. input/output)
        let storage_class = inferred_storage_class_from_ty
            .or(attr_storage_class)
            .unwrap_or_else(|| match (is_ref, mutbl) {
                (false, _) => StorageClass::Input,
                (true, hir::Mutability::Mut) => StorageClass::Output,
                (true, hir::Mutability::Not) => self.tcx.sess.span_fatal(
                    hir_param.ty_span,
                    &format!(
                        "invalid entry param type `{}` (expected `{}` or `&mut {1}`)",
                        layout.ty, value_ty
                    ),
                ),
            });

Should this just call the span_fatal if the storage calls was not inferred?

[rust-gpu-bot]

Comment from msiglreith (CONTRIBUTOR) on 2021-11-24T17:59:33Z

---

@andrewleverette I think this only refers to the #spirv(..) attributes which can be explicitly specified by the user and not the storage classes as a whole as they are still needed (just inferred by compiler).
https://github.com/EmbarkStudios/rust-gpu/blob/d789b58b2e3f57bdd1cfc30ae4fed98e736b9577/crates/rustc_codegen_spirv/src/symbols.rs#L138-L165

[rust-gpu-bot]

Comment from andrewleverette on 2021-11-24T18:08:18Z

---

Okay so sorry if this is a dumb question, but is this issue literally just removing those specified attributes from STORAGE_CLASSES?

[rust-gpu-bot]

Comment from andrewleverette on 2021-11-24T18:36:20Z

---

I think the linked pull request will resolve this issue. I'm still new to open source, so I'm definitely open to any feedback.