Skip to content

Imm representation ideas #29

Description

@Firestar99

From Rust-GPU/rust-gpu#379 (comment) since @eddyb asked me to open a separate issue about it:

From what I could gleam from spirt source:
[...]

  • Imm::Short(u32) is sufficient for all usual operands, as they are all just u32-sized enums
  • Exception: string literals, for which you use one Imm::LongStart with many Imm::LongCont[inued] to encode a stream of N-many bytes
    • I have not found any other case using Long

Thoughts

  • With Imm::Long* rarely used but Imm::Short ubiquitous, I wonder if an Imm::unwrap_short() could make code more readable? And maintainable, since it would allow you to replace the backing datastructure later without much code breakage.
  • With Imm::Long being so rare and I assume untouched by spirt, I wonder if a plain Vec<u8> or &'cx [u8] is more efficient and simpler? That would also make OpString only have 2 Imms, which makes it fit in Inst.imms: SmallVec<[Imm; 2]> and only be one indirection to access, as previously.
pub enum Imm {
    Short(spec::OperandKind, u32),
    Long(spec::OperandKind, Vec<u32>),
}

Another idea from Imm docs:

spirt/src/spv/mod.rs

Lines 97 to 98 in 43524d7

// FIXME(eddyb) consider replacing with a `struct` e.g.:
// `{ first: bool, last: bool, kind: OperandKind, word: u32 }`

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions