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 an read_clock_khr function that calls OpReadClockKHR #757

Merged
merged 10 commits into from
Oct 8, 2021

Conversation

expenses
Copy link
Contributor

Reading from the shader clock lets us create heatmaps for ray tracing as seen here: https://developer.nvidia.com/blog/profiling-dxr-shaders-with-timer-instrumentation/.

heatmap

crates/rustc_codegen_spirv/src/spirv_type_constraints.rs Outdated Show resolved Hide resolved
crates/spirv-std/src/arch.rs Outdated Show resolved Hide resolved
crates/spirv-std/src/arch.rs Show resolved Hide resolved
crates/spirv-std/src/arch.rs Outdated Show resolved Hide resolved
@khyperia
Copy link
Contributor

Please also add compiletests

@expenses
Copy link
Contributor Author

Hmm, compiletests doesn't seem to like those #[cfg] flags.

@khyperia
Copy link
Contributor

... oh, ugh, right, since we build spirv-std without those flags, but the test itself with those flags (something that never happens, and cannot happen, in the real world - can only happen here since we're directly invoking rustc instead of going through cargo). uuuuh, hmm, going to try to think of a way to resolve it...

@tomachinz
Copy link

nanosecond timing will be useful.

Timestamp values in this time domain are in units of nanoseconds and are comparable with platform timestamp values captured using the POSIX clock_gettime API. This extension advertises the SPIR-V ShaderClockKHR capability for Vulkan, which allows a shader to query a real-time or monotonically incrementing counter at the subgroup level or across the device level. The two valid SPIR-V scopes for OpReadClockKHR are Subgroup and Device.

@khyperia
Copy link
Contributor

khyperia commented Oct 4, 2021

This is a horrible no-good very bad solution, and I'm really hoping I can think of something better, but right now my only thought to fix it is adding ShaderClockKHR and the ext to here.

@expenses
Copy link
Contributor Author

expenses commented Oct 4, 2021

This is a horrible no-good very bad solution, and I'm really hoping I can think of something better, but right now my only thought to fix it is adding ShaderClockKHR and the ext to here.

That doesn't look too too bad to me :)

Another think that I thought off that could to have a seperate feature flag just for the compiletests:

#[cfg(any(
    all(
        target_feature = "Int64",
        target_feature = "ShaderClockKHR",
        target_feature = "ext:SPV_KHR_shader_clock"
    ),
    target_feature = "compiletests"
)]

That way we don't have to shove a whole lot of features into that line you linked.

@expenses
Copy link
Contributor Author

expenses commented Oct 8, 2021

@khyperia Happy to go with either idea, or to put this on-hold for a bit.

@khyperia
Copy link
Contributor

khyperia commented Oct 8, 2021

Whatever you think is best works!

@expenses
Copy link
Contributor Author

expenses commented Oct 8, 2021

Another think that I thought off that could to have a seperate feature flag just for the compiletests:

#[cfg(any(
    all(
        target_feature = "Int64",
        target_feature = "ShaderClockKHR",
        target_feature = "ext:SPV_KHR_shader_clock"
    ),
    target_feature = "compiletests"
)]

That way we don't have to shove a whole lot of features into that line you linked.

Actually, that doesn't work. We just run into an invalid capability error:

if let Some(input) = input.strip_prefix(EXT_PREFIX) {
Ok(Self::Extension(Symbol::intern(input)))
} else {
Ok(Self::Capability(input.parse().map_err(|_err| {
format!("Invalid Capability: `{}`", input)
})?))
}

@expenses expenses requested a review from khyperia October 8, 2021 12:45
@khyperia khyperia merged commit 14e2152 into EmbarkStudios:main Oct 8, 2021
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.

5 participants