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

[Aleksey Shipilev]

> Compare:
> 
> void Assembler::cmpq(Address dst, Register src) {
>   InstructionMark im(this);
>   emit_int16(get_prefixq(dst, src), 0x3B);
>   emit_operand(src, dst);
> }
> 
> void Assembler::cmpq(Register dst, Address src) {
>   InstructionMark im(this);
>   emit_int16(get_prefixq(src, dst), 0x3B);
>   emit_operand(dst, src);
> }
> 
> They use the same opcode -- `0x3B`, which is for `CMP r, r/m`. While `cmpq(Address,Register)` actually should be using `0x39` for `CMP r/m, r`. I also suspect they emit basically the same instruction, because the `get_prefixq` and `emit_operand` argument order is irrelevant.
> 
> AFAIU, it does not break horribly, because the `cmpq(Address,Register)` is not used anywhere except the new code in `MacroAssembler::safepoint_poll`, added by [JDK-8253180](https://bugs.openjdk.java.net/browse/JDK-8253180). This was found by Zhengyu, when he tried to enable that new code on x86_32 by inverting `cmpq(addr, reg); jcc(above, slow_path)` to `cmpptr(reg, addr); jcc(belowEquals, slow_path)`. Then, everything blew up, because the semantics of `cmpq(addr,reg)` was wrong, and this inversion was subtly broken.
> 
> Current candidate patch encodes this `cmpq` properly. Since that changes the semantics, I had to flip the condition code in its only use. I opted to do this, because _maybe_ some code in downstream projects want to use this odd `cmpq`. Although even if so, the uses could be trivially rewritten.
> 
> Alternatives:
>  - I considered removing `cmpq(Address,Register)` altogether, but it would require more work to untangle `cmpptr(Address,Register)` and `cmpptr(Address,AddressLiteral)` for x86_32. 
>  - We can also split out `MacroAssembler::safepoint_poll` change to use `cmpq(Register,Address)` to begin with, but current shape gives us a way to test the encoding.
> 
> Additional testing:
>  - [x] tier1 with Shenandoah (a few failures are pre-existing)
>  - [x] tier1 with Z (AFAICS, all failing tests are OOME'ing or break SA, and probably are problem-listed)

Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:

 - Merge branch 'master' into JDK-8255550-cmpq-incorrect
 - 8255550: x86: Assembler::cmpq(Address dst, Register src) encoding is incorrect

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

Changes: https://git.openjdk.java.net/jdk/pull/910/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=910&range=01
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/910.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/910/head:pull/910

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