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

Martin Doerr mdoerr at openjdk.java.net
Mon Feb 21 14:16:53 UTC 2022


On Wed, 2 Feb 2022 14:40:58 GMT, Lutz Schmidt <lucy at openjdk.org> wrote:

> May I please request reviews for this cleanup and unification change. It streamlines the address operand emitters for all instructions. As such, the modifications are widespread and may be cumbersome to review. Please ask questions if anything is unclear.
> 
> SAP does not maintain a full s390x build and test environment any longer. I would therefore very much appreciate if somebody would run a build and test cycle on s390x. 
> 
> As this is a s390x-only change, other platforms are not impacted.

Looks basically good. I have a few minor remarks. Did you test this carefully? I can't guarantee that there's no typo or copy&paste bug.

src/hotspot/cpu/s390/assembler_s390.hpp line 1812:

> 1810:   inline void emit_data(int x);
> 1811: 
> 1812: // private:

Uncommon. Should better get removed.

src/hotspot/cpu/s390/assembler_s390.inline.hpp line 52:

> 50:         *(unsigned short*)(pos)   = (unsigned short)(x>>16);
> 51:         *(unsigned short*)(pos+2) = (unsigned short)x;
> 52:       }

Looks correct, but what is the motivation behind splitting unaligned stores? z can store to unaligned memory.

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.

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.

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

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


More information about the hotspot-compiler-dev mailing list