New validation of RSP vector instructions seems to incorrectly mark some cases as invalid

[Mr-Wiseguy]

In a code base that previously worked and matched some known binaries, new versions of armips after commit a53c5f8a9d7f9520be2a25bd8c22f5c5b5a953ea are unable to assemble a matching binary. It seems that some vector instructions (slv in this case) are now enforcing a vector index alignment that is more strict than required. See https://github.com/Mr-Wiseguy/f3dex2/issues/1 for more details.

[sp1187]

This seems odd considering that the Nintendo 64 RSP Programmers Guide lists the valid element values for llv and slv as 0, 4, 8 and 12, which is violated by your disassembly. Could this be a mistake in the original F3DEX implementation somehow? If so, it would interesting to know what the actual hardware behavior is for this instruction, is it just ignoring the least significant bits or is something more complex happening here?

As this could be an issue for future disassembly projects, maybe the validation should be reverted?

[Mr-Wiseguy]

I'd be willing to check how the instruction behaves on actual hardware but I don't know of a way to debug microcode so I'm not sure how I could go about doing it. Perhaps looking at the source of a low-level graphics plugin like Angrylion or ParaLLEl may yield some info?

[sp1187]

The easiest way might be just tweaking one of krom's tests (https://github.com/peterlemon/n64) and run on a console.

[rasky]

All elements values for llv/lsv (and all vector loads/stores) are valid. They are just undocumented in official Nintendo docs. I documented behavior of most of them here: https://github.com/rasky/r64emu/blob/master/doc/rsp.md

(reversed and tested on real N64). The assembler should allow them as they are valid.

[sp1187]

The new validation now reverted by https://github.com/Kingcom/armips/commit/a40cb8ca677dde89fc149917d935cdf590a56095 as some of the "invalid" ones was indeed shown to be useful. It is still possible that a less strict validation could be added some time in the future, but would have to take a deeper look at the behavior of the undocumented/"invalid" values. Also note that RSP instructions can never crash the RSP, so there are nothing that should be validated for that reason.

[clbr]

Please make it a warning then. Misaligned element is in 99% of cases a bug.

[rasky]

I think it depends on the context. I'm writing ucode that uses that, and I'm surely don't want a fixed warning forever. What is the context in which using that is a bug?

I'm not sure how it is a bug more than any other typo in the assembly code; the fact that it isn't documented in SGI docs doesn't seem too indicative -- those docs are full of mistakes anyway. Maybe @sp1187 might want to add an opt-in warning for undocumented opcodes, in case somebody wants to limit themselves to using documented opcodes (though I wouldn't know why).

[clbr]

It's the opposite of "not documented". See page 49 of the SGI RSP Programmers guide, they are actively documented as invalid.

I filed the bug because I hit it (or almost hit); just like many other bugs I filed here. For example, you're operating on 64-bit data and doing bit-manip on them. You fat-finger the element load. Bug!

It's very much intended for loading the same-sized data; it's really the special case of misusing them to load data to the "wrong" location that should need an op to disable the warning.

[clbr]

In fact, I'd like the disable-warning-op to be per-line. An annotation, "misaligned-element-load", added to the load that does so, to keep the warning active for all other loads in the file.

[unknownbrackets]

This isn't the only place with this problem, technically.

https://github.com/Kingcom/armips/blob/a40cb8ca677dde89fc149917d935cdf590a56095/Archs/MIPS/CMipsInstruction.cpp#L240

Branches shouldn't be in delay slots, but "can't" is a strong word. In fact, they can and are, even in commercial games:

https://github.com/hrydgard/ppsspp/issues/1926 https://github.com/hrydgard/ppsspp/pull/3175

To me it seems like a place for a directive, like .allowinvalid and .disallowinvalid or something.

-[Unknown]

[rasky]

I tested this on RSP and only the last branch of the sequence is effectively executed, the others are ignored without side effects. In this case, those instructions are effectively rendered NOPs so personally I would suggest to emit a warning in this case.

LLV for unaligned elements is more of a philosophical discussion since the instruction works correctly and the behavior is coherent and well-defined. So it's up to the maintainer to decide this.