KhronosGroup / KhronosGroup/SPIRV-LLVM-Translator

Remove support for the internal SPIR-V text format

Open
#211 4 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
LLVM
Stars
625
Forks
279
Avg merge
3d 5h
Merged PRs (30d)
34

Description

The translator currently supports its own textual format for SPIR-V assembly. This format is incompatible with the official format handled by the SPIRV-Tools. It is furthermore poorly tested, probably doesn't support all the corner cases and is generally a liability to the translator code base.

The translator currently produces or consumes this format via three different options:
- `--spirv-text` for a translation from IR to the SPIR-V text format
- `--to-binary` to assemble a SPIR-V text file to a SPIR-V binary
- `--to-text` to disassemble a SPIR-V binary to a SPIR-V text file

I'm tempted to suggest that none of this functionality belongs in the translator and that we should get rid of it entirely, using the relevant SPIR-V Tools for testing.

A possible staged approach could be to:
- [ ] Require SPIR-V Tools as a hard dependency and remove the optionality of SPIR-V validation (correctness shouldn't be an option :)) in the testing.
- [ ] Remove support for `--to-text` and use `spirv-dis`. There are 11 uses in tests.
- [ ] Remove support for `--to-binary` and use `spirv-as`. There are 17 uses in tests.
- [ ] Remove support for `--spirv-text` completely (125 uses in tests) and use `spirv-dis` in testing. The various name map data structures could be kept for in-debugger use (I know @neildhickey has been thinking about this).

These changes would make it easier to reuse code from SPIR-V tools as suggested in #32.

Contributor guide

Open the contributing guide

Research direction

Start by tracing the three options named in the issue: --spirv-text, --to-binary, and --to-text, then inspect their 11, 17, and 125 test uses. Review the proposed SPIR-V Tools replacements, including spirv-dis and spirv-as, and determine whether the testing dependency should become mandatory. Done means the internal text-format paths and their test usage are removed without leaving the translator dependent on them.

Written by the indexing model from the issue text.

Assessment

Domain
compilers
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.