RFR: 8281079: [s390] Unify Address Operand Encoding in Instruction Emitters

[Martin Doerr]

On Tue, 22 Feb 2022 08:05:57 GMT, Lutz Schmidt <lucy at openjdk.org> wrote:

>> src/hotspot/cpu/s390/assembler_s390.inline.hpp line 1412:
>> 
>>> 1410:   int len_bits = instr >> (48-2);
>>> 1411:   if (len_bits == 0) len_bits = instr >> (32-2);
>>> 1412:   if (len_bits == 0) len_bits = instr >> (16-2);
>> 
>> Tricky, but looks correct.
>
> Want some detailed comment here?

I can understand it, but a short comment would be appreciated. E.g. length can only be >2 if first 2 bits are != 0.

>> src/hotspot/cpu/s390/assembler_s390.inline.hpp line 1423:
>> 
>>> 1421:       // The switch expression examines just the leftmost two bytes
>>> 1422:       // of the main opcode. So the range of values is just [0..3].
>>> 1423:       // Having a default clause makes the compiler happy.
>> 
>> A bit lengthy for a trivial explanation.
>
> You mean I should not explain what's going on?

The explanation in the middle is fine. What's the value of the first and last sentence? The first one is already expressed by ShouldNotReachHere, the last one is a general statement which I don't like to be at every switch.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7324