pretty: prefer shrinking instead of thinning text, and make SPIR-V ops/enums more distinct. #21
Conversation
This file contains 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
oisyn
previously approved these changes
Feb 17, 2023
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
eddyb
force-pushed
the
pretty-style-improvements
branch
from
cbfbba6
to
948bab2
Compare
Was able to use |
eddyb
added a commit
that referenced
this pull request
Apr 21, 2023
…->lift passes. (#24) #### `qptr` is the first major SPIR-T feature/departure from SPIR-V, and represents a vision for memory/pointers: * name stands for "`q`uasi-`p`oin`t`e`r`" <sub><sup>(alternatively, "`q`uantum")</sup></sub>, in the sense of "not quite (a pointer)" * <sub>(primarily chosen to distinguish it from both SPIR-V pointers and LLVM-style untyped `ptr`s)</sub> * **untyped** _as a design principle_ ("behavior given by operations, not types") * maximally so, not even tracking the address space ("Storage Class"), unlike e.g. LLVM `ptr`s * comparable to OpenCL `Generic` physical pointers (but more flexible and without runtime behavior) * **for Rust-GPU**, this allows faithfully representing Rust's *untyped memory* semantics * <sub>(Rust also lacks explicit address spaces, and `qptr` can replace Rust-GPU's "Storage Class" inference pass)</sub> * **for legalization/optimization**, existing approaches (from IRs with true pointers) can be employed * **takes advantage** of SPIR-V's "logical"/"physical" dichotomy to **maximize** flexibility and **minimize** effort: * <ins>**"logical pointers"**</ins>: strictly typed, but extremely limited dataflow <sub><sup>(without `VariablePointers`)</sup></sub> * as they <sub><sup>(without `VariablePointers`)</sup></sub> can't be stored to memory, returned from functions or passed through `OpPhi`s, each "logical pointer" is strictly a *static alias* of some variable, with a maybe-dynamic offset * <sub>(calls allow taking "logical pointer" arguments, but only shallowly, and the pointers have to point to a whole variable, so it's more like such functions are "templated" over the choice of variable)</sub> * the limitations allow *guaranteed* recovery of pointee types (and "Storage Class"es) for *legal* usage * <sub>this is because both forwards (def->use) and backwards (use->def) dataflow can connect each definition to its *static* "tree" of uses (with e.g. `qptr.offset` nodes, and `qptr.load`/`qptr.store` leaves), and all such "trees" are *disjoint* between pointer definitions - such a "tree"-like structure can be seen reified in the `qptr.usage` attribute (produced by `qptr::analyze`)</sub> * *illegal* usage, *crucially*, **must** be legalized away to ever produce legal SPIR-V, so there is no need to ever support recreating SPIR-V pointers from anything more than the (very restricted) legal subset * <ins>**"physical pointers"**</ins>: still typed, but support casts and have no dataflow restrictions * SPIR-V copy of [LLVM's old `T addrspace(AS)*` pointer types](https://releases.llvm.org/14.0.0/docs/LangRef.html#pointer-type) (via OpenCL SPIR) * can be treated as untyped by erasing the pointee type, and keeping only the "Storage Class" * <sub>(the "Storage Class" only needs to be tracked on `int2ptr` casts, loads of physical pointers etc. - *not implemented yet*)</sub> * dedicated operations (`QPtrOp` - shown as `qptr.*` below) * <sub>(as a fallback, for other SPIR-V instructions, attributes record original pointee types required for an input, or the output - for output pointers, the SPIR-V "Storage Class" is also kept)</sub> * basic pass pipeline: * **`qptr::lower`**: SPIR-V `OpTypePointer` types/instructions are lowered to `qptr` and `qptr.*` ops * `OpAccessChain`s on composite types become pointer arithmetic (erasing those types) * <sub>(long-term Rust-GPU can skip this step and emit `qptr` operations directly)</sub> * [this is where any future legalization passes would go] * **`qptr::analyze`**: `qptr` uses are analyzed to generate `qptr.usage` attributes * <sub>(by merging compatible uses, this can itself legalize *some* `union`-like uses that were illegal in the SPIR-V input of `qptr::lower` - Rust-GPU could use this to support many more shapes of `enum`s)</sub> * **`qptr::lift`**: `qptr.*` ops are lifted back to SPIR-V `OpTypePointer` types/instructions * `qptr.usage` attributes are used to generate pointee types that support all (transitive) uses * <sub>(by propagating accurate "Storage Class"es, this can itself legalize *some* incorrect "Storage Class"es in the SPIR-V input of `qptr::lower` - Rust-GPU could use this to replace its "Storage Class" inference pass)</sub> --- #### `qptr`s can point to either: * <ins>**handles**</ins>: one or more textures, samplers, *or **buffers*** * name [inspired by WGSL](https://gpuweb.github.io/gpuweb/wgsl/#address-space) (which sadly only uses it for textures/samplers) * **handle arrays** use `qptr.handle_array_index` to select a single handle (i.e. "descriptor indexing") * **buffers** are opaque handles, and use `qptr.buffer_data` to obtain a `qptr` to the *memory* contents * (comparable to using an `OpImageTexelPointer` in SPIR-V, on a pointer to an `OpTypeImage`) * `qptr.buffer_dyn_len` can query the size of dynamically-sized buffers (replacing `OpArrayLength`) * intentionally replacing SPIR-V's choice of using a mix of `Block`-decorated `OpTypeStruct`s in specific "Storage Class"es <sub>(`Uniform`, `StorageBuffer`, `ShaderRecordBufferKHR`)</sub> to encode buffers "syntactically" (i.e. mimicking their GLSL declaration syntax) * <ins>**memory**</ins>: byte-addressable *untyped memory* * the memory is either defined in the shader (global/local vars), or obtained from handles etc. * pointer arithmetic uses `qptr.offset` and `qptr.dyn_offset` * again more untyped than LLVM (whose GEP was copied by SPIR-V into `OpAccessChain`) * to avoid separate multiplications, `qptr.dyn_offset` includes an immediate stride <sub>(in the extreme this could be generalized to a kind of "integer dot product", if N-dimensional arrays ever require it)</sub> * reading/writing typed values from/to memory uses `qptr.load`/`qptr.store` --- #### Example (`qptr` passes on `kajiya`'s [`assets/shaders/wrc/wrc_see_through.rgen.hlsl`](https://github.com/EmbarkStudios/kajiya/blob/5a124d58b8fc8795d37c933168c4ce98e986ce4c/assets/shaders/wrc/wrc_see_through.rgen.hlsl)):  *(type information like `OpMemberName` can be seen to be erased, and only used offsets are recreated)*  *(`qptr.*` ops can be seen as blue against the backdrop of orange `spv.*` ops, thanks to #21)* --- _**FIXME(@eddyb):** move/copy some of this description into the library documentation (maybe a design document?)_
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The main observation here is that since picking most of the SPIR-T pretty-printing styles, I've started using RA inlays set to a tiny size, e.g.:
It's probably a more subjective choice, but on a 4K monitor the result is quite crisp.
In contrast to this, SPIR-T styles used a lot of "low visibility" but never shrinking text, which now feels worse.
On the other hand, I've been working on some experiments (involving pointers), and some instructions and attributes unique/specific to SPIR-T, and when those are not visually distinguishable from SPIR-V, it can get confusing.
This PR tries to tackle a bit of all that, the result is:
(click for complete
pretty HTML example)
(click for complete
pretty HTML example)
(because of the sheer length of those gray comment lines, I couldn't even put them side-by-side)
What I think is a clear win:
.instead of leaving it the default colorFooinFoo.Barwhen the.isn't sticking out like a sore thumb)FunctionControl.(.Inline)became justFunctionControl.InlineWhat I'm less sure about:
spv.everywhere - it's cute, but it might overstay its welcomeOp...get an orange color? (e.g. attributes are still green right now)spv.)spv.means theOp...could take different colors (e.g. blue in types/consts), potentiallyspv.OpVariable<spv.StorageClass.Function>to become justOpVariable<Function><...>(...)could make sense in most situationsIn the end, this PR introduces more textual verbosity, and that's probably to be preferred, I just wish I already had an easy toggle for "verbosity levels" (e.g. using a tiny bit of JS to toggle CSS classes).