Omr: Should byte-reversed load/store opcodes be moved to OMR?

[aviansie-ben]

It seems that there are a number of IL opcodes in OpenJ9 meant to perform byte-swapped indirect loads and stores (irsload, iriload, irlload, irsstore, iristore, and irlstore). These could be useful for implementing languages that strictly require that certain loads/stores be performed with a specific endianness. For instance, WebAssembly specifies that all linear memory operations must be performed as if the system were little-endian [1], since it's possible to alias memory loads/stores of different types (e.g. loading a byte from a value stored as an integer).

As far as I can tell, these opcodes are completely language-independent and do not carry any Java semantics with them. Additionally, there's already some code in OMR for dealing with these opcodes in the Power codegen that is currently guarded by J9_PROJECT_SPECIFIC [2]. I'd argue that these factors weigh in favour of moving these opcodes to OMR to allow their use in other projects and to reduce the amount of J9_PROJECT_SPECIFIC-guarded code present in this repository.

[1] https://webassembly.github.io/spec/core/exec/numerics.html#storage
[2] For example: https://github.com/eclipse/omr/blob/980032feed19ea524ab7602582219ea389628e25/compiler/p/codegen/FPTreeEvaluator.cpp#L78-L85

[fjeremic]

There was a discussion at one point on whether we should deprecate the reverse load opcodes because not every architecture supports them and instead use the TR::byteswap IL [1] on top of a normal load to model this scenario. Codegens which have reverse load instructions can easily recognize such patterns without having to introduce another set of load IL which the optimizer can choke on.

I can't seem to find this discussion anymore. Perhaps it was even internal before we open sourced.

[1] https://github.com/eclipse/omr/blob/c9e04c2bd059abc784af9970f597c612f65681e1/compiler/il/OMRILOpCodeProperties.hpp#L11856-L11870

[aviansie-ben]

@fjeremic Good point. I did look around for previous discussion on this topic, but wasn't able to find any myself. Avoiding special IL opcodes for these cases could help get rid of quite a bit of the weirder code surrounding these opcodes. However, it seems that we don't currently have sbyteswap and lbyteswap opcodes, so those would need to be added to cover all cases that the byte-reversed loads/stores are currently used in (though we should probably do that regardless).

[0xdaryl]

I also remember having a discussion on this, but perhaps it was stitching together comments made across several issues and PRs over the years.

I do have the text of a proposal that I will attribute to @Leonardo2718 :-) that I was in favour of at the time:

Remove reverse load/store opcodes

The reverse load and reverse store opcodes add a fair amount of complexity to the IL while providing relatively little benefit. Removing these will simplify the IL while also allowing J9_PROJECT_SPECIFIC code to be removed.

The approach should be to first identify the instructions that will replace these (likely load/store followed by swap). Once identified, the code can be modified to use them. If these do not yet exist, they may need to be implemented.

Note: removing these opcodes raises some performance concerns that must be addressed.

We have an issue already for lbyteswap (#958). It should probably be extended to include the short type for completeness.