RFR: 8241042: x86_64: Improve Assembler generation

Claes Redestad claes.redestad at oracle.com
Mon Mar 16 23:43:22 UTC 2020



On 2020-03-16 23:29, Vladimir Kozlov wrote:
> Okay, emit_int* code change seems fine (I did not verify each and every 
> line - hope testing will show correctness).

Thanks!

> 
> About get_prefixq() change.
> 
> 1. Would be nice to have common prefixes table and method get_prefixq(). 
> Could be done by passing noreg (-1) >
> void Assembler::prefixq(Address adr) {
>    emit_int8(get_prefixq(adr, noreg));
> }

Hmm, get_prefixq(adr, reg) calls reg->encoding(), which will assert on
noreg (assert(reg->is_valid(), ...)).

> 
> 2. Use asserts for them as you did in first get_prefixq() variant.

This is a good idea, in fact I thought I already had the
corresponding assert block in both places.

> 
> 3. May swap values in table by reversion (src->encoding() < 8) condition.

Hmm, maybe we should just exploit the fact that the REX_W* values
returned in both cases are consecutive from REX_W to REX_WXB and
REX_WRXB respectively, and ordered in a way that makes it possible to
get rid of the lookup tables entirely:

int8_t Assembler::get_prefixq(Address adr) {
   int8_t prfx = (int8_t)(REX_W + ((int)adr.base_needs_rex()) + 
((int)adr.index_needs_rex() << 1));
   ..

int8_t Assembler::get_prefixq(Address adr, Register src) {
   int8_t prfx = (int8_t)(REX_W + ((int)adr.base_needs_rex()) + 
((int)adr.index_needs_rex() << 1) + ((int)(src->encoding() >= 8) << 2));
   ..

http://cr.openjdk.java.net/~redestad/8241042/open.02/

While this is really a bit too clever, the assertions make for a very
strong fail-safe, and I think all the possibilities should be checked
rather promply by our regular testing. I've sanity tested locally and
verified the code gen look a bit better without the lookup tables.

Running tier1-4 over night.

Thanks!

/Claes


More information about the hotspot-compiler-dev mailing list