print: use `spv.OpFoo(A: imm, B: ID, C: imm, ...)` instead of `spv.OpFoo<imms>(IDs)`. #33

eddyb · 2023-06-08T02:49:35Z

There's two main changes here:

removal of all angle brackets (<...>) from SPIR-T pretty-printing
- they used to be used for immediates (almost as if they were "const generics", to distinguish them from regular inputs/arguments, which used parens)
- may have had some use in quickly visualizing what the "immediates"/"inputs" split may be, but I don't recall actually relying on that, and it's probably better to have SPIR-T-"native" forms for any aspects of SPIR-V we care about
- new style uses the standard SPIR-V order (interleaving immediates and ID operands)
- long term this may make it easier to parse a SPIR-T textual form (but there are no current plans for that)
introduction of "named argument" labels (foo: before an operand)
- for SPIR-V ops and enumerands, this should help with readability, especially when dealing with a larger number of operands (e.g. for image/raytracing ops)
- with immediates interleaved with other operands, this felt potentially helpful in reducing ambiguities that could've been introduced (when the <...> distinction was removed, specifically)
- where overly noisy this again introduces an incentive to have SPIR-T-"native" forms

Comparison (using Kajiya's assets/shaders/raster_simple_ps.hlsl, compiled by DXC):

Before _{(click for complete pretty HTML example)}	After _{(click for complete pretty HTML example)}

…dth back to 120. (#37) *Originally found/fixed during the #33/#34/#35/#36 series of pretty-printing PRs.* Before this PR, we were treating all text the same, regardless of style, but this PR introduces the concept of "fractional columns" (integer multiples of `0.1ch`, because our `font-size`s are always integer multiples of `0.1em`). The result is much more accurate, and so far appears to perfectly match browser behavior (i.e. the transition of `MAX_LINE_WIDTH` from `N-1` to `N`, where multi-line layout gets replaced with single-line layout, results in a line that "tightly fits" in a `max-width: Nch` container, with no spurious gaps or overflows). Note however that the increased accuracy makes the layout favor HTML over plaintext (and we don't have a plaintext-vs-HTML "simultaneous layout", nor a way to hint what the output of pretty layout will be used for), so the plaintext output may feel more inconsistent as a result (but its aesthetics are of dubious value anyway).

…printed SPIR-T is up to date in CI. (#34) *Originally found while working on #33, and also used to contain #37 before splitting that off.* The pretty-printing fix here (not showing `OpTypePointer` that are used internally in `GlobalVarDecl`/`PtrToGlobalVar` but never actually meant to be printed) is relatively small, the main change is the new CI check, which should ensure we don't forget to update `README.md`.

…(IDs)`.

…y_spv_inst`.

… ops.

… syntax. (#35) This moves us from my ad-hoc `#{A, B, C}` to a Rust-style `#[A] #[B] #[C]` attribute syntax, and removes the idea of a "attribute set shorthand" (`attrs123 = #{...}` followed by uses of `#attrs123`), which was more annoying than useful (attributes can't refer to other attributes so there was no "exponential deduplication" need for it). With `#[...]` syntax, it also makes sense to never say e.g. `OpDecorate` (as the operand itself will contain the word `Decoration`), or to use `#[name = "..."]` Rust-style syntax for `OpName`. (I didn't touch `OpMember{Name,Decorate}`, might be worth having a SPIR-T `struct` type declaration syntax just to clean those up but it feels even more ad-hoc than usual - maybe it might make more sense after #36?) Comparison (using Kajiya's `assets/shaders/raster_simple_ps.hlsl`, compiled by DXC, like #33): |[**Before**](https://htmlpreview.github.io/?https://gist.github.com/eddyb/124d807950059893678ad55f51ee5107/raw/0-before-spirt%252335-raster_simple_ps.hlsl.structured.spirt.html) (click for complete pretty HTML example)|[**After**](https://htmlpreview.github.io/?https://gist.github.com/eddyb/124d807950059893678ad55f51ee5107/raw/1-after-spirt%252335-raster_simple_ps.hlsl.structured.spirt.html) (click for complete pretty HTML example)| |-|-| |![image](https://github.com/EmbarkStudios/spirt/assets/77424/82ee846a-0ed7-4f6b-be4f-7c6ccd1ba2fa)|![image](https://github.com/EmbarkStudios/spirt/assets/77424/f725b9bd-7715-4c96-8e4e-e573e41a8478)|

This was referenced Jun 8, 2023

print: hide spurious OpTypePointers, and ensure README.md pretty-printed SPIR-T is up to date in CI. #34

Merged

print: accurately track (fractional) line width, and raise the max width back to 120. #37

Merged

eddyb added 4 commits June 13, 2023 14:58

print: use spv.OpFoo(imm, ID, imm, ...) instead of `spv.OpFoo<imms>…

4b0bc8f

…(IDs)`.

print: deduplicate spv::print::Token handling and streamline `prett…

f510021

…y_spv_inst`.

print: add (deemphasized) name: "named arguments" labels to SPIR-V…

6584847

… ops.

Update CHANGELOG.

72bacba

eddyb force-pushed the print-parens branch from 2210f5b to 72bacba Compare June 13, 2023 21:27

eddyb marked this pull request as ready for review June 13, 2023 21:27

eddyb enabled auto-merge June 13, 2023 21:27

eddyb added this pull request to the merge queue Jun 13, 2023

Merged via the queue into main with commit a21e9f2 Jun 13, 2023

eddyb deleted the print-parens branch June 13, 2023 21:37

eddyb mentioned this pull request Jun 13, 2023

print: always print attributes inline and with per-attribute #[...] syntax. #35

Merged

eddyb mentioned this pull request Jul 24, 2023

Updated SPIR-T for better pretty-printing (esp. OpExtInst), and started deduplicating custom debuginfo. EmbarkStudios/rust-gpu#1083

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

print: use `spv.OpFoo(A: imm, B: ID, C: imm, ...)` instead of `spv.OpFoo<imms>(IDs)`. #33

print: use `spv.OpFoo(A: imm, B: ID, C: imm, ...)` instead of `spv.OpFoo<imms>(IDs)`. #33

eddyb commented Jun 8, 2023 •

edited

Loading

print: use spv.OpFoo(A: imm, B: ID, C: imm, ...) instead of spv.OpFoo<imms>(IDs). #33

print: use spv.OpFoo(A: imm, B: ID, C: imm, ...) instead of spv.OpFoo<imms>(IDs). #33

Conversation

eddyb commented Jun 8, 2023 • edited Loading

print: use `spv.OpFoo(A: imm, B: ID, C: imm, ...)` instead of `spv.OpFoo<imms>(IDs)`. #33

print: use `spv.OpFoo(A: imm, B: ID, C: imm, ...)` instead of `spv.OpFoo<imms>(IDs)`. #33

eddyb commented Jun 8, 2023 •

edited

Loading