[aarch64-port-dev ] RFR: JDK-8184900 AArch64: Fix overflow in immediate cmp instruction

[Yang Zhang]

Hi Andrew

I have updated the patch based on your feedback.

http://cr.openjdk.java.net/~yzhang/8184900/webrev.01/

Please check it again.

Regards
Yang

On 25 July 2017 at 16:34, Andrew Haley <aph at redhat.com> wrote:
> On 25/07/17 05:46, Yang Zhang wrote:
>> Hi, all
>>
>> For a fastdebug build openjdk in jdk10/hs tree, there is a failed test
>> case (TestOptionsWithRanges) in jtreg. The reason is that the
>> immediate cmp instruction has 12 significant bits. This causes an
>> overflow in TestOptionsWithRanges.
>>
>> With this patch, TestOptionsWithRanges can be passed and there aren't
>> new failed test cases in jtreg.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8184900
>>
>> Webrev:
>> http://cr.openjdk.java.net/~yzhang/8184900/webrev.00/
>>
>> Please help to review it.
>
> This comment makes no sense to me:
>
> +  // The imm should follow aarch64 add/sub imm12 constraint.
>    inline void cmp(Register Rd, unsigned imm)  { subs(zr, Rd, imm); }
>
> Please just say "imm is limited to 12 bits."
>
>    inline void cmnw(Register Rd, unsigned imm) { addsw(zr, Rd, imm); }
> diff --git a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
> --- a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
> +++ b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
> @@ -764,7 +764,8 @@
>        // alignment.
>        Label small;
>        int low_limit = MAX2(zva_length * 2, (int)BlockZeroingLowLimit);
> -      __ cmp(cnt, low_limit >> 3);
> +      Register tmp = rscratch1;
> +      __ subs(tmp, cnt, low_limit >> 3);
>
> Please don't declare Register tmp if it's only used once.
>
> Otherwise fine, thanks.
>
> --
> Andrew Haley
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671