Improve PSP VFPU instruction parsing & add tests#255
Open
tclamb wants to merge 2 commits into
Open
Conversation
Owner
|
Thanks a lot! This looks very nice. The original implementation of those instructions definitely didn't get enough testing... I think I mostly tried to copy what I saw in PPSSPP at the time. Just one small thing, can you add a quick note at the top of the changelog in the Readme summarizing the breaking changes, maybe with a link to the documentation? Under a |
- reworked `vfim.s` to accept float & int literals (previously took integer representations of IEEE754 `float32`s, e.g. `0x3f800000` for `1.0`)
- removed support for the invalid `vsbn.{p,t,q}` instructions
- fixed `vt` size for `vscl.{p,t,q}` (is always scalar, regardless of `vs` & `vd`)
- fixed `vd` size for the various pack/unpack instructions
- renamed `vuc2i.s` to `vuc2ifs.s`
- fixed various issues with `vpfx{s,t,d}` parsing
- the sequence parser is greedy, so `1` would match before fractions like `1/3` (fixed)
- numeric literals were not matched at all since the parser only expected identifier tokens (fixed)
- requiring the operands to be wrapped with `[`/`]` breaks compatibility with other tooling (removed)
- `m` is a write mask operand that prevents writing to a certain destination slot, not a modifier on a saturation operation (fixed)
- `0-1`/`-1-1` syntax for the `vpfxd` saturation operations `[0:1]`/`[-1:1]` is not supported elsewhere, and the unbracketed `0:1`/`-1:1` syntax is not supported by MWCC (removed)
- `vpfxd` supports unmodified slots with an empty operand
- fixed `0` support in `vrot.{p,t,q}` (had expected an identifier token, not an integer)
Used MWCC to generate expected output for the added/modified tests.
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 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.
I ran into a few issues with VFPU functions while iterating on mods. It seems like tokenizer changes may have broken a few of these instructions at some point? This commit fixes some discrepancies I found with the pspdev vfpu documentation, aligns prefix syntax with binutils/MWCC, and adds tests to prevent future regressions.
vfim.sto accept float & int literals (previously took integer representations of IEEE754float32s, e.g.0x3f800000for1.0)vsbn.{p,t,q}instructionsvtsize forvscl.{p,t,q}(is always scalar, regardless ofvs&vd)vdsize for the various pack/unpack instructionsvuc2i.stovuc2ifs.svpfx{s,t,d}parsing1would match before fractions like1/3(fixed)[/]breaks compatibility with other tooling (removed)mis a write mask operand that prevents writing to a certain destination slot, not a modifier on a saturation operation (fixed)0-1/-1-1syntax for thevpfxdsaturation operations[0:1]/[-1:1]is not supported elsewhere, and the unbracketed0:1/-1:1syntax is not supported by MWCC (removed)0support invrot.{p,t,q}(had expected an identifier token, not an integer)I used MWCC to generate expected output for the added/modified tests.