This repository was archived by the owner on Oct 31, 2025. It is now read-only.
Infer Storage Classes by "specializing" SPIR-V modules "generic" over them.#414
Merged
eddyb merged 16 commits intoEmbarkStudios:mainfrom Feb 22, 2021
LykenSol:specializer
Merged
Infer Storage Classes by "specializing" SPIR-V modules "generic" over them.#414eddyb merged 16 commits intoEmbarkStudios:mainfrom LykenSol:specializer
eddyb merged 16 commits intoEmbarkStudios:mainfrom
LykenSol:specializer
Conversation
Contributor
Author
|
I think I just realized I'm not doing Result ID renumbering inside expanded functions, a bit surprised that didn't blow up in my face yet - I'm guessing the inliner is resilient to that kind of issue, since it just does its own renumbering. |
…phization-like expansion.
khyperia
approved these changes
Feb 22, 2021
|
|
||
| [lib] | ||
| crate-type = ["dylib"] | ||
| test = false |
Contributor
There was a problem hiding this comment.
what does this do? there are a few tests in rustc_codegen_spirv/linker/test.rs, does this disable those?
Contributor
Author
There was a problem hiding this comment.
I... this is an accident, sorry! I was trying to debug reducing my RLS waiting time (it didn't work sigh) and accidentally included it in a commit apparently (wouldn't have happened if I was still using git gui oh well).
This was referenced Feb 22, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
How It Works
OpTypePointers are always emitted byrustc_codegen_spirvwith a placeholder Storage Classrspirv::drin-memoryGeneric, which may be confusing, and also it has different semantics than what we're doing, but as long as we don't need the realGenericStorage Class, we're goodPlaceholderbelow (maybeUnknownwould be better?)Input/Output/PushConstant, etc.) variables used by entry-points: thankfully,OpVariablehas a redundant Storage Class, so we can set that to the correct value, but keep the pointer typeGenericspirv_std::storage_class::*types have no effect anymore on SPIR-V types, they're only used to pick the (Input/Output/PushConstant, etc.) Storage Class of interfaceOpVariablesDerefand be indistinguishable from plain references!%13 = OpTypePointer Placeholder %12is treated as if it were%13<$0> = OpTypePointer $0 %12, where$0denotes the first (and only, in this example) generic parameter of the instruction%14 = OpTypeStruct %13 %13 %13becomes%14<$0, $1, $2> = OpTypeStruct %13<$0> %13<$1> %13<$2>- this would come from something like(&T, &T, &T): same pointee type, but other than that the pointers are independentOpFunctions, but they are exactly as "generic" as theirOpTypeFunction(i.e. signature), ignoring the body (see below for what role instructions in the function body, eventually play)OpConstant*s, globalOpVariables, and function bodiesspirv_type_constraints, which was introduced in inline asm!: support writing _ in lieu of return types, for basic inference. #376, and augmented in the first 3 commits of this PRrustc's ownenalibrary, but we don't need its features, so it's just extra complexity)sky-shader's entrypoint):?0,?1,?2,?3are inference variables, created because of how generic theOpFunctionCallarguments are->is before, and<-is after, inference - the reason?2and?3are known right away is because%717and%718are (interface) globalOpVariables that have already been inferred to always needInput, andOutput, respectivelyfound Match [...]line shows the type/Storage Class equalities coming from applying thespirv_type_constraintspatterns, that will then be acted upon to either make some inference variables "aliases" of others (via union-find), or outright assign a known Storage Class to them?0and?1are now also known, but more importantly,%714(the callee) now has concrete (Storage Class) "generic arguments" for its "generic parameters"InputStorage Class%13<Input>or%14<Function, PushConstant, Function>, and in the realistic (sky-shader) example, we have%714<Input, Output>(as well as%717<Input>and%718<Output>)Replacements(i.e. operand positions and what to replace the operand at that position with)Pre-Merge Checklist
renumber IDs inside functions
look for and/or address remaining
// TODO(eddyb)comments in the codeimplement matching/inference support for
OpAccessChain& friends (usingTyPat::IndexComposite)%2717<?1, ?2>.0 = %2544<?6, ?7>which results in?6 = ?1and?7 = ?2)take advantage of Add a % for Operand::IdRef Display impl gfx-rs/rspirv#184 to simplify some debug logging
add
Derefimpls tospirv_std::storage_class::*or replace them entirely (see Investigate inferring storage class #300)test the
rust-gpu-shadertoysexamples with this PR, as they may stress it further than the in-tree ones,and attempt to remove the need to copyconstmatrices to local variables (in order to call.transpose())test with no DCE pass before the
specializerpassmouse-shader, with DCE before specializer:(post-Use [profile.release.build-override] to optimize build dependencies. #437)
link_dcespecialize_generic_storage_classlink_inlinelink_dce(post-Use [profile.release.build-override] to optimize build dependencies. #437)
specialize_generic_storage_classlink_inlinelink_dcefigure out if I want to keep the commit history (and therefore convince someone to not merge this PR with squashing)
Closes #300.