Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom_insts for our own OpExtInsts, and use it for some debuginfo. #1064

Merged
merged 11 commits into from
Jun 1, 2023

Conversation

eddyb
Copy link
Contributor

@eddyb eddyb commented May 26, 2023

This PR introduces a Rust-GPU-specific custom "extended instruction set" (OpExtInstImport/OpExtInst - note that unlike the requirements imposed by the SPIR-V specification, we're only concerned about rspirv/spirt compatibility, as long as we always rewrite such instructions before the final SPIR-V output).

Longer-term, we'll likely want such custom instructions for more advanced usecases (e.g. one idea being floated around was adding our own OpAbort that gets transformed to standard SPIR-V), and the debuginfo needs will likely get superseded by adopting NonSemantic.Shader.DebugInfo.100, but for now this is easier for some debuginfo.

The main feature this PR adds is the ability to have location ranges, not just the starting point:

before this PR (since #1040)after this PR
error: cannot memcpy dynamically sized data
    --> $CORE_SRC/intrinsics.rs:2460:9
     |
2460 |         copy(src, dst, count)
     |         ^
     |
note: used from within `core::intrinsics::copy::`
    --> $CORE_SRC/intrinsics.rs:2447:21
     |
2447 | pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) {
     |                     ^
note: called by `ptr_copy::copy_via_raw_ptr`
    --> $DIR/ptr_copy.rs:28:18
     |
28   |         unsafe { core::ptr::copy(src, dst, 1) }
     |                  ^
note: called by `ptr_copy::main`
    --> $DIR/ptr_copy.rs:33:5
     |
33   |     copy_via_raw_ptr(&i, o);
     |     ^
note: called by `main`
    --> $DIR/ptr_copy.rs:31:1
     |
31   | #[spirv(fragment)]
     | ^
error: cannot memcpy dynamically sized data
    --> $CORE_SRC/intrinsics.rs:2460:9
     |
2460 |         copy(src, dst, count)
     |         ^^^^^^^^^^^^^^^^^^^^^
     |
note: used from within `core::intrinsics::copy::`
    --> $CORE_SRC/intrinsics.rs:2447:21
     |
2447 | pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) {
     |                     ^^^^
note: called by `ptr_copy::copy_via_raw_ptr`
    --> $DIR/ptr_copy.rs:28:18
     |
28   |         unsafe { core::ptr::copy(src, dst, 1) }
     |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: called by `ptr_copy::main`
    --> $DIR/ptr_copy.rs:33:5
     |
33   |     copy_via_raw_ptr(&i, o);
     |     ^^^^^^^^^^^^^^^^^^^^^^^
note: called by `main`
    --> $DIR/ptr_copy.rs:32:8
     |
32   | pub fn main(i: f32, o: &mut f32) {
     |        ^^^^

The other feature is inline stack frames, e.g. most of this --no-early-report-zombies output is new:
(actually, it used to not even report the zombies, because the decorations were being lost)

error: cannot memcpy dynamically sized data
    --> $CORE_SRC/intrinsics.rs:2460:9
     |
2460 |         copy(src, dst, count)
     |         ^^^^^^^^^^^^^^^^^^^^^
     |
note: used from within `core::intrinsics::copy::`
    --> $CORE_SRC/intrinsics.rs:2460:9
     |
2460 |         copy(src, dst, count)
     |         ^^^^^^^^^^^^^^^^^^^^^
note: called by `ptr_copy::copy_via_raw_ptr`
    --> $DIR/ptr_copy.rs:28:18
     |
28   |         unsafe { core::ptr::copy(src, dst, 1) }
     |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: called by `ptr_copy::main`
    --> $DIR/ptr_copy.rs:33:5
     |
33   |     copy_via_raw_ptr(&i, o);
     |     ^^^^^^^^^^^^^^^^^^^^^^^
note: called by Fragment entry-point `main`
    --> $DIR/ptr_copy.rs:32:8
     |
32   | pub fn main(i: f32, o: &mut f32) {
     |        ^^^^

error: cannot memcpy dynamically sized data --> $CORE_SRC/intrinsics.rs:2460:9 | 2460 | copy(src, dst, count) | ^^^^^^^^^^^^^^^^^^^^^ | note: used from within `ptr_copy::copy_via_raw_ptr` --> $DIR/ptr_copy.rs:11:4 | 11 | fn copy_via_raw_ptr(src: &f32, dst: &mut f32) { | ^^^^^^^^^^^^^^^^ note: called by `ptr_copy::main` --> $DIR/ptr_copy.rs:38:5 | 38 | copy_via_raw_ptr(&src, &mut dst); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: called by Fragment entry-point `main` --> $DIR/ptr_copy.rs:32:8 | 32 | pub fn main(i: f32, o: &mut f32) { | ^^^^

The more flexible debuginfo is only used internally, and replaced with standard SPIR-V OpLine/OpNoLine instructions (or rather, the SPIR-T attribute that spirt::spv::lift maps to OpLine/OpNoLine) after all possible (zombie or SPIR-T) diagnostics have been emitted, to maximize our diagnostics quality.

@eddyb eddyb marked this pull request as ready for review June 1, 2023 19:43
@eddyb eddyb enabled auto-merge (rebase) June 1, 2023 21:07
@eddyb eddyb requested review from fu5ha, repi and VZout June 1, 2023 21:08
Copy link
Member

@VZout VZout left a comment

Choose a reason for hiding this comment

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

fancy!

also learned some new rust :p if let [Operand::IdRef(target), ..] = inst.operands[..] {

@eddyb eddyb merged commit 87b7d13 into EmbarkStudios:main Jun 1, 2023
@eddyb eddyb deleted the extinst branch June 1, 2023 22:05
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