[12] RFR 8206895: aarch64: rework error-prone cmp instruction

[Boris Ulasevich]

On 11.07.2018 18:48, Andrew Haley wrote:
> On 07/11/2018 04:42 PM, Boris Ulasevich wrote:
>> Please review the following patch:
>> http://cr.openjdk.java.net/~bulasevich/8206895/webrev.02
>> https://bugs.openjdk.java.net/browse/JDK-8206895
>>
>> cmp instruction overloading introduced to force user to specify
>> immediate value type implicitly. The only allowed type is unsigned char.
> 
> But how does this solve the problem?  Won't the C++ compiler silently
> truncate cmp(reg, int) to a uint8 ?

With overloaded cmp function compiler complains on any cmp(reg, imm) 
usage. The only way to use cmp is to implicitly specify parameter type. 
Once user specified unsigned char type (other types are not allowed), it 
is user responsibility to ensure the value is correct:

cmp(reg, 8);    // gcc: call of overloaded cmp is ambiguous
cmp(reg, 7722); // gcc: call of overloaded cmp is ambiguous
cmp(reg, (u1)8);    // ok
cmp(reg, (u1)7722); // user error

>> For immediate values outside of byte range subs macro should be called
>> directly.
>>
>> cmp calls all over the codes was fixed accordingly to the following plan:
>>     cmp(reg, imm8)  -> cmp(reg, (u1)imm8)
>>     cmp(reg, imm12) -> subs(zr, reg, imm12)
>>     cmp(reg, imm64) -> subs(scratch, reg, imm64)
>>
>> The change was tested with jtreg tests.
> 
> This hunk looks wrong:
> 
> @@ -1129,7 +1129,7 @@
> 
>     if (super_check_offset.is_register()) {
>       br(Assembler::EQ, *L_success);
> -    cmp(super_check_offset.as_register(), sc_offset);
> +    subs(zr, super_check_offset.as_register(), sc_offset);
> 
> cmp with register is still allowed.
  Yes, cmp(reg,reg) is Ok. But sc_offset is immediate value:
int sc_offset = in_bytes(Klass::secondary_super_cache_offset());