RFR: 8292638: x86: Improve scratch register handling in VM stubs [v4]

[Aleksey Shipilev]

On Sat, 20 Aug 2022 01:13:19 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> (PR depends on #9923.)
>> 
>> Constants which reside outside code cache are not guaranteed to be reachable in
>> RIP-relative addressing mode from stub code, but absolute addressing mode requires a
>> scratch register.
>> 
>> Provide a scratch register to ensure constants are reachable irrespective of
>> process memory layout. 
>> 
>> Make scratch register usage explicit to make possible conflicts with surrounding stub code explicit.
>> 
>> Testing:
>> - [x] hs-tier1 - hs-tier4
>> - [x] x86_32 hotspot build
>
> Vladimir Ivanov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
> 
>  - Merge branch 'master' into trig_rscratch
>  - rscratch
>  - cmp64
>  - 8292638: x86: Improve scratch register handling in VM stubs
>  - StubRoutines cleanup
>  - tabs
>  - x86: Improve handling of constants in trigonometric stubs

Some review comments. Let me run x86_32 tests real quick.

src/hotspot/cpu/x86/assembler_x86.cpp line 12240:

> 12238:   if (!is_reachable) {
> 12239:     assert(!always_reachable(adr), "sanity");
> 12240:   }

Maybe a bit cleaner like this:

Suggestion:

  assert(is_reachable || !always_reachable(adr), "sanity");

src/hotspot/cpu/x86/assembler_x86.cpp line 12244:

> 12242: }
> 12243: 
> 12244: bool Assembler::always_reachable(AddressLiteral adr) {

So, there is a similar block in `Assembler::is_reachable_from` -- does it make sense to reuse it then? They also are subtly different on the use of `ForceUnreachable`, would this method return different result compared to `Assembler::is_reachable_from`, and thus fail the assert in `Assembler::reachable`?

src/hotspot/cpu/x86/macroAssembler_x86.cpp line 525:

> 523: void MacroAssembler::cmp64(Register src1, AddressLiteral src2, Register rscratch) {
> 524:   assert(!src2.is_lval(), "should use cmpptr");
> 525:   assert(rscratch != noreg || always_reachable(src2),  "missing");

Here and later, redundant double space before "missing".

src/hotspot/cpu/x86/macroAssembler_x86.hpp line 1272:

> 1270:   void movsd(Address     dst, XMMRegister    src) { Assembler::movsd(dst, src); }
> 1271:   void movsd(XMMRegister dst, XMMRegister    src) { Assembler::movsd(dst, src); }
> 1272:   void movsd(XMMRegister dst, Address        src)     { Assembler::movsd(dst, src); }

Minor misindent introduced.

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

PR: https://git.openjdk.org/jdk/pull/9932