RFR: 8253191: C2: Masked byte comparisons with large masks produce wrong result on x86

[Tobias Hartmann]

On Wed, 7 Oct 2020 10:39:16 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

> `testUB_mem_imm` generates erroneous code when `mask` constant is larger than `Byte.MAX_VALUE`.
> 
> AD instruction in question:
> instruct testUB_mem_imm(rFlagsReg cr, memory mem, immU8 imm, immI0 zero) %{
>   match(Set cr (CmpI (AndI (LoadUB mem) imm) zero));
> 
>   ins_encode %{ __ testb($mem$$Address, $imm$$constant); %}
> 
> The following instruction sequence is problematic:
> testb  $0x80,0x10(%rdi,%r9,1)
> jle    0x00000001168789a0
> 
> It performs *signed* byte comparison and the immediate is interpreted as a negative value.
> 
> The original code shape was as follows:
> movzbl 0x10(%rcx,%r9,1),%r9d
> test   $0x80,%r9d
> jle    0x000000010a9b6a00
> 
> The fix is to narrow the range of accepted mask constants and set the upper limit to `Byte.MAX_VALUE`.
> 
> Testing: hs-precheckin-comp, hs-tier1, hs-tier2.

Looks good.

test/hotspot/jtreg/compiler/c2/TestUnsignedByteCompare.java line 48:

> 46:     @DontInline static boolean testByteLT0(byte[] val) { return (val[0] & mask()) <  0; }
> 47:
> 48:     static void testValue(byte b) {

Shouldn't you exclude that method from compilation to compare interpreted vs. C2 compiled result?

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

Marked as reviewed by thartmann (Reviewer).

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