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

[Andrew Haley]

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