Skip to content

rspirv2 disassembly - #632

Open
Firestar99 wants to merge 4 commits into
mainfrom
rspirv2_dis
Open

rspirv2 disassembly#632
Firestar99 wants to merge 4 commits into
mainfrom
rspirv2_dis

Conversation

@Firestar99

@Firestar99 Firestar99 commented Jul 30, 2026

Copy link
Copy Markdown
Member

We released rspirv2 🎉

  • best reviewed commit by commit
  • last commit "switch dis preset to default" has a ton of bless changes that don't need to be reviewed, since I'm only changing the disassembly settings for prettier disassembly
  • rather, review the blesses in "replace disassembler*" where actual differences between rspirv and rspirv2 are visible
  • while you're reviewing this, please also approve ci: split compiletest target-envs into steps #533

@Firestar99
Firestar99 force-pushed the rspirv2_dis branch 2 times, most recently from f6f9bea to 7d58b22 Compare July 30, 2026 13:45
@Firestar99
Firestar99 marked this pull request as ready for review July 31, 2026 09:34
}

use rspirv::binary::Disassemble;
use rspirv::binary::{Assemble, Disassemble};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rspirv2 doesn't have its own traits here?

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 assume you mean the function arg impl Assemble? That just needs to be rspirv's Assemble (can even drop Disassemble) to accept both rspirv's Module (compiletests with disassemble and disassemble-global) or Function (disassemble-function) as args. And turn those into bytes that rspirv2 can then "parse" (since it decodes on the fly, it doesn't actually parse it) and disassemble.

Comment thread crates/rustc_codegen_spirv/src/codegen_cx/mod.rs Outdated
.unwrap();
module
.dis(DisOptions {
color: false,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Color should be taken from the session context's --color value

@Firestar99 Firestar99 Aug 4, 2026

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.

Nope we can't. compiletest-rs would compare our output with ascii sequences to the *.stderr file without any, and fail every test. (Though it does print pretty colors while doing so :D )

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants