RFR: 8255550: x86: Assembler::cmpq(Address dst, Register src) encoding is incorrect

[Aleksey Shipilev]

On Thu, 29 Oct 2020 07:37:35 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>>> Thanks for poking me. I would prefer to change to the cmpq instruction that has the opposite order in the stack watermark barrier instead. Everywhere in the code I talk about the condition being sp being "above" watermark. Changing it to less makes me twist my head in ways that heads should not twist.
>> 
>> Ok, so let's do this: can you change the parameter order in `cmpq` instruction in `safepoint_poll`, run the tests you probably know are sensitive to this, and I'll drop the `safepoint_poll` hunk from here after that change lands?
>
>> > Thanks for poking me. I would prefer to change to the cmpq instruction that has the opposite order in the stack watermark barrier instead. Everywhere in the code I talk about the condition being sp being "above" watermark. Changing it to less makes me twist my head in ways that heads should not twist.
>> 
>> Ok, so let's do this: can you change the parameter order in `cmpq` instruction in `safepoint_poll`, run the tests you probably know are sensitive to this, and I'll drop the `safepoint_poll` hunk from here after that change lands?
> 
> What I meant was "in a separate PR", not to mess up with the change here. I think it amounts to:
> 
> diff --git a/src/hotspot/cpu/x86/macroAssembler_x86.cpp b/src/hotspot/cpu/x86/macroAssembler_x86.cpp
> index a8da3aa17b8..81303ea76c4 100644
> --- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp
> +++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp
> @@ -2765,7 +2765,7 @@ void MacroAssembler::safepoint_poll(Label& slow_path, Register thread_reg, bool
>    if (at_return) {
>      // Note that when in_nmethod is set, the stack pointer is incremented before the poll. Therefore,
>      // we may safely use rsp instead to perform the stack watermark check.
> -    cmpq(Address(thread_reg, Thread::polling_word_offset()), in_nmethod ? rsp : rbp);
> +    cmpq(in_nmethod ? rsp : rbp, Address(thread_reg, Thread::polling_word_offset()));
>      jcc(Assembler::above, slow_path);
>      return;
>    }
> 
> I can do that, if you want, and if you trust `tier1` is enough.

Forked `safepoint_poll` change to JDK-8255579, #924

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

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