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@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