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

[Vladimir Ivanov]

On Thu, 8 Oct 2020 08:22:48 GMT, Tobias Hartmann <thartmann 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.
>
> 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?

I could do that, but it's not required: `testUB_mem_imm` matches memory operand, but `testValue` does
`(b & mask())` while `testByte...` do `val[0] & mask()`. So, `testUB_mem_imm` is used only in `testByte...` methods.

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

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