[aarch64-port-dev ] aarch64: Fix a bug in TestOptionsWithRanges of hotspot

[Yang Zhang]

Hi

Could you please help to review the updated patch as follows:

---

diff --git a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
index f8e4c85..e5e437a 100644
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
@@ -4853,7 +4853,12 @@
   // alignment.
   if (!is_large || !(BlockZeroingLowLimit >= zva_length * 2)) {
     int low_limit = MAX2(zva_length * 2, (int)BlockZeroingLowLimit);
-    cmp(cnt, low_limit >> 3);
+    if (operand_valid_for_add_sub_immediate(low_limit >> 3)) {
+      cmp(cnt, low_limit >> 3);
+    } else {
+      mov(tmp, low_limit >> 3);
+      cmp(cnt, tmp);
+    }
     br(Assembler::LT, small);
   }

diff --git a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
index 6ca67ac..2b7cb26 100644
--- a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
+++ b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
@@ -2346,8 +2346,14 @@
     __ subw(count, count, cnt_words, Assembler::LSL, 3 - shift);
     if (UseBlockZeroing) {
       Label non_block_zeroing, rest;
+      Register tmp = rscratch1;
       // count >= BlockZeroingLowLimit && value == 0
-      __ cmp(cnt_words, BlockZeroingLowLimit >> 3);
+      if (Assembler::operand_valid_for_add_sub_immediate(BlockZeroingLowLimit
>> 3)) {
+        __ cmp(cnt_words, BlockZeroingLowLimit >> 3);
+      } else {
+        __ mov(tmp, BlockZeroingLowLimit >> 3);
+        __ cmp(cnt_words, tmp);
+      }
       __ ccmp(value, 0 /* comparing value */, 0 /* NZCV */, Assembler::GE);
       __ br(Assembler::NE, non_block_zeroing);
       __ mov(bz_base, to);

Ps. I also attach the patch file.

On 30 June 2016 at 16:45, Yang Zhang <yang.zhang at linaro.org> wrote:
> I have modified the patch according your comments. I am testing the new patch.
> When it's okay, I will send it again.
>
> Regards
> Yang
>
> On 28 June 2016 at 18:09, Stuart Monteith <stuart.monteith at linaro.org> wrote:
>> From my reading of this - BlockZeroingLowLimit is the N bytes that dc zva
>> would write in a single operation - zva_length.
>> I can see a case for the minimum of being zva_length, but the maximum
>> shouldn't have a limit really - and nothing as small as you are suggesting.
>> That the cmp immediate limit is smaller than we like is incidental to the
>> actual limit we'd be aiming for. Say, for example, there was only an
>> advantage on some machines/workloads above 2 MB.
>>
>> Can you look into reworking the "cmp" call to something more permissive?
>>
>> Thanks,
>>
>>    Stuart
>>
>>
>> On 28 June 2016 at 09:40, Yang Zhang <yang.zhang at linaro.org> wrote:
>>>
>>> In file src/cpu/aarch64/vm/stubGenerator_aarch64.cpp:2350,
>>> BlockZeroingLowLimit is used as follows:
>>> __ cmp(cnt_words, BlockZeroingLowLimit >> 3);
>>>
>>> On aarch64 platform, the immediate in cmp instruction has 12
>>> significant bits which could be 0xXXX or 0xXXX000. The immediate is
>>> calculated by BlockZeroingLowLimit >> 3. So BlockZeroingLowLimit has
>>> 15 significant bits. It's low limit, and a vary large value isn't
>>> meaningful. So the range of BlockZeroingLowLimit should be [1, 2^15 -
>>> 1].
>>>
>>> On 28 June 2016 at 16:16, Andrew Haley <aph at redhat.com> wrote:
>>> > On 28/06/16 05:09, Yang Zhang wrote:
>>> >> Could someone please help to review and process attached patch for
>>> >> fixing
>>> >> an issue about BlockZeroingLowLimit in hotspot openjdk9 aarch64?
>>> >>
>>> >> This issue is exposed by a test failure in hotspot jtreg openjdk9 for
>>> >> aarch64
>>> >>
>>> >> runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java
>>> >>
>>> >> It is caused by improper range of BlockZeroingLowLimit.
>>> >
>>> > Surely this is just papering over the real bug.
>>> >
>>> > Andrew.
>>> >
>>
>>
-------------- next part --------------
From dc921fbd4b224bbc8c2c00bd99f41a0dc08895a5 Mon Sep 17 00:00:00 2001
From: Yang Zhang <yang.zhang at linaro.org>
Date: Fri, 24 Jun 2016 15:12:49 +0800
Subject: [PATCH] aarch64: Fix a bug in TestOptionsWithRanges of hotspot

On aarch64 platform, the immediate in cmp instruction has 12 significant
bit. When BlockZeroingLowLimit is out of range, move it to register.

Change-Id: Ie581830da2e77278c12df4042f516ecae4560802
---

diff --git a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
index f8e4c85..e5e437a 100644
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
@@ -4853,7 +4853,12 @@
   // alignment.
   if (!is_large || !(BlockZeroingLowLimit >= zva_length * 2)) {
     int low_limit = MAX2(zva_length * 2, (int)BlockZeroingLowLimit);
-    cmp(cnt, low_limit >> 3);
+    if (operand_valid_for_add_sub_immediate(low_limit >> 3)) {
+      cmp(cnt, low_limit >> 3);
+    } else {
+      mov(tmp, low_limit >> 3);
+      cmp(cnt, tmp);
+    }
     br(Assembler::LT, small);
   }
 
diff --git a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
index 6ca67ac..2b7cb26 100644
--- a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
+++ b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
@@ -2346,8 +2346,14 @@
     __ subw(count, count, cnt_words, Assembler::LSL, 3 - shift);
     if (UseBlockZeroing) {
       Label non_block_zeroing, rest;
+      Register tmp = rscratch1;
       // count >= BlockZeroingLowLimit && value == 0
-      __ cmp(cnt_words, BlockZeroingLowLimit >> 3);
+      if (Assembler::operand_valid_for_add_sub_immediate(BlockZeroingLowLimit >> 3)) {
+        __ cmp(cnt_words, BlockZeroingLowLimit >> 3);
+      } else {
+        __ mov(tmp, BlockZeroingLowLimit >> 3);
+        __ cmp(cnt_words, tmp);
+      }
       __ ccmp(value, 0 /* comparing value */, 0 /* NZCV */, Assembler::GE);
       __ br(Assembler::NE, non_block_zeroing);
       __ mov(bz_base, to);