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