RFR: 8241042: x86_64: Improve Assembler generation

[Vladimir Kozlov]

Okay, emit_int* code change seems fine (I did not verify each and every line - hope testing will show correctness).

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));
}

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

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

Thanks,
Vladimir

On 3/16/20 1:09 PM, Claes Redestad wrote:
> 
> 
> On 2020-03-16 18:51, Claes Redestad wrote:
>>>>
>>>> on x64, folding consecutive bytes emitted by Assembler routines into
>>>> byte-wise variants of emit_int16, emit_int24 and emit_32 turns out to
>>>> be profitable, since gcc can't seem to fold the back to back stores
>>>> to the end() pointer into one. The new methods are also a bit
>>>> convenient.
>>>
>>> Yes, I agree with this.
>>>
>>> Can you put () around arguments like next? It would be easy see them:
>>>
>>> emit_int24((0x44 | regenc), (scale << 6 | indexenc | baseenc), (disp & 0xFF));
>>
>> Sure.
>>
>>>
>>> Or better put them on separate lines as you did in some cases.
>>
>> I prefer keeping the expressions more compact, unless there's a need to
>> comment on what's going on.
>>
>>>
>>> Can we avoid cast (unsigned char) or should we do it always (when sign bit may be set)? I seems we added it to avoid 
>>> additional instructions generated by gcc. 
> 
> I've made a number of edits that should help with readability in places
> where it got a bit muddied, especially in emit_operand:
> 
> http://cr.openjdk.java.net/~redestad/8241042/open.01/
> 
> Also made use of (unsigned char) a bit more consistent.
> 
> I think a follow up to examine if we can get rid of these casts by
> changing arguments to uint8_t might be worthwhile.
> 
> Rerunning tier1+2.
> 
> /Claes