-
Notifications
You must be signed in to change notification settings - Fork 244
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
Infer Storage Classes by "specializing" SPIR-V modules "generic" over them. #414
Conversation
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.
@@ -11,6 +11,7 @@ repository = "https://github.com/EmbarkStudios/rust-gpu" | |||
|
|||
[lib] | |||
crate-type = ["dylib"] | |||
test = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this do? there are a few tests in rustc_codegen_spirv/linker/test.rs
, does this disable those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
How It Works
OpTypePointer
s are always emitted byrustc_codegen_spirv
with a placeholder Storage Classrspirv::dr
in-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 realGeneric
Storage Class, we're goodPlaceholder
below (maybeUnknown
would be better?)Input
/Output
/PushConstant
, etc.) variables used by entry-points: thankfully,OpVariable
has a redundant Storage Class, so we can set that to the correct value, but keep the pointer typeGeneric
spirv_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 interfaceOpVariable
sDeref
and be indistinguishable from plain references!%13 = OpTypePointer Placeholder %12
is treated as if it were%13<$0> = OpTypePointer $0 %12
, where$0
denotes the first (and only, in this example) generic parameter of the instruction%14 = OpTypeStruct %13 %13 %13
becomes%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 independentOpFunction
s, 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, globalOpVariable
s, 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 ownena
library, but we don't need its features, so it's just extra complexity)sky-shader
's entrypoint):?0
,?1
,?2
,?3
are inference variables, created because of how generic theOpFunctionCall
arguments are->
is before, and<-
is after, inference - the reason?2
and?3
are known right away is because%717
and%718
are (interface) globalOpVariable
s that have already been inferred to always needInput
, andOutput
, respectivelyfound Match [...]
line shows the type/Storage Class equalities coming from applying thespirv_type_constraints
patterns, 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?0
and?1
are now also known, but more importantly,%714
(the callee) now has concrete (Storage Class) "generic arguments" for its "generic parameters"Input
Storage 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 = ?1
and?7 = ?2
)take advantage of Add a % for Operand::IdRef Display impl gfx-rs/rspirv#184 to simplify some debug logging
add
Deref
impls tospirv_std::storage_class::*
or replace them entirely (see Investigate inferring storage class #300)test the
rust-gpu-shadertoys
examples with this PR, as they may stress it further than the in-tree ones,and attempt to remove the need to copyconst
matrices to local variables (in order to call.transpose()
)test with no DCE pass before the
specializer
passmouse-shader
, with DCE before specializer:(post-Use [profile.release.build-override] to optimize build dependencies. #437)
link_dce
specialize_generic_storage_class
link_inline
link_dce
(post-Use [profile.release.build-override] to optimize build dependencies. #437)
specialize_generic_storage_class
link_inline
link_dce
figure out if I want to keep the commit history (and therefore convince someone to not merge this PR with squashing)
Closes #300.