[aarch64-port-dev ] RFR: suppress implicit null check when offsets too large for immediate

[Andrew Haley]

Hi,

On 03/19/2014 05:41 PM, Edward Nevill wrote:

> The following patch suppresses the null pointer check if the offset
> is too large to fit in a single load/store.
> 
> This is necessary because in this case MacroAssembler::form_address
> can insert extra instructions before the load/store and the null
> pointer check can end up pointing to the wrong instruction.
> 
> For simplicity in this patch I suppress the implicit null pointer
> check with any offset >= 1024 (ie I just pass 0 as the shift value
> to offset_ok_for_immed). The actual shift value does not seem to be
> readily available and I do not believe it will make any significant
> difference.
> 
> This is not a pretty solution and I think it needs to be revised in
> the future with a better structured approach to handle this. However
> for now this will solve the problem.

I found the root cause of this problem.  The operand types used by the
memory access instructions in aarch64.ad are wrong.

For example,

-operand indIndexScaledOffsetI(iRegP reg, iRegL lreg, immIScale scale, immIAddSub off)
+operand indIndexScaledOffsetI(iRegP reg, iRegL lreg, immIScale scale, immIU12 off)

Here we see that an immIAddSub is being used.  This is wrong: it
allows more ranges than a scaled offset instruction, which only
accepts an unsigned 12-bit scaled offset.  I've been through
aarch64.ad and fixed all of them, I hope.

Andrew.


# HG changeset patch
# User aph
# Date 1395332256 14400
#      Thu Mar 20 12:17:36 2014 -0400
# Node ID 16e2ffe3b357aa5e6f7f255fd19777fa6c21a9c5
# Parent  dd34c2dac1b81d174648b8dbcc667f39f2819b82
Correct operand perdicates used by load and store operands.

diff -r dd34c2dac1b8 -r 16e2ffe3b357 src/cpu/aarch64/vm/aarch64.ad
--- a/src/cpu/aarch64/vm/aarch64.ad	Thu Mar 20 10:13:24 2014 -0400
+++ b/src/cpu/aarch64/vm/aarch64.ad	Thu Mar 20 12:17:36 2014 -0400
@@ -3547,16 +3547,34 @@
   interface(CONST_INTER);
 %}

-// Valid 9 bit unsigned offset
-operand immI9()
-%{
-  predicate(((-(1 << 8)) <= n->get_int()) && (n->get_int() < (1 << 8)));
+operand immLU12()
+%{
+  predicate((0 <= n->get_long()) && (n->get_long() < (1 << 12)));
+  match(ConL);
+
+  format %{ %}
+  interface(CONST_INTER);
+%}
+
+// Offset for scaled or unscaled immediate loads and stores
+operand immIOffset()
+%{
+  predicate(Address::offset_ok_for_immed(n->get_int()));
   match(ConI);

   format %{ %}
   interface(CONST_INTER);
 %}

+operand immLoffset()
+%{
+  predicate(Address::offset_ok_for_immed(n->get_long()));
+  match(ConL);
+
+  format %{ %}
+  interface(CONST_INTER);
+%}
+
 // 32 bit integer valid for add sub immediate
 operand immIAddSub()
 %{
@@ -4115,7 +4133,7 @@
   %}
 %}

-operand indIndexScaledOffsetI(iRegP reg, iRegL lreg, immIScale scale, immIAddSub off)
+operand indIndexScaledOffsetI(iRegP reg, iRegL lreg, immIScale scale, immIU12 off)
 %{
   constraint(ALLOC_IN_RC(ptr_reg));
   match(AddP (AddP reg (LShiftL lreg scale)) off);
@@ -4129,7 +4147,7 @@
   %}
 %}

-operand indIndexScaledOffsetL(iRegP reg, iRegL lreg, immIScale scale, immLAddSub off)
+operand indIndexScaledOffsetL(iRegP reg, iRegL lreg, immIScale scale, immLU12 off)
 %{
   constraint(ALLOC_IN_RC(ptr_reg));
   match(AddP (AddP reg (LShiftL lreg scale)) off);
@@ -4143,7 +4161,7 @@
   %}
 %}

-operand indIndexScaledOffsetI2L(iRegP reg, iRegI ireg, immIScale scale, immLAddSub off)
+operand indIndexScaledOffsetI2L(iRegP reg, iRegI ireg, immIScale scale, immLU12 off)
 %{
   constraint(ALLOC_IN_RC(ptr_reg));
   match(AddP (AddP reg (LShiftL (ConvI2L ireg) scale)) off);
@@ -4199,7 +4217,7 @@
   %}
 %}

-operand indOffI(iRegP reg, immIAddSub off)
+operand indOffI(iRegP reg, immIOffset off)
 %{
   constraint(ALLOC_IN_RC(ptr_reg));
   match(AddP reg off);
@@ -4213,7 +4231,7 @@
   %}
 %}

-operand indOffL(iRegP reg, immLAddSub off)
+operand indOffL(iRegP reg, immLoffset off)
 %{
   constraint(ALLOC_IN_RC(ptr_reg));
   match(AddP reg off);
@@ -4243,7 +4261,7 @@
   %}
 %}

-operand indIndexScaledOffsetIN(iRegN reg, iRegL lreg, immIScale scale, immIAddSub off)
+operand indIndexScaledOffsetIN(iRegN reg, iRegL lreg, immIScale scale, immIU12 off)
 %{
   predicate(Universe::narrow_oop_shift() == 0);
   constraint(ALLOC_IN_RC(ptr_reg));
@@ -4258,7 +4276,7 @@
   %}
 %}

-operand indIndexScaledOffsetLN(iRegN reg, iRegL lreg, immIScale scale, immLAddSub off)
+operand indIndexScaledOffsetLN(iRegN reg, iRegL lreg, immIScale scale, immLU12 off)
 %{
   predicate(Universe::narrow_oop_shift() == 0);
   constraint(ALLOC_IN_RC(ptr_reg));
@@ -4273,7 +4291,7 @@
   %}
 %}

-operand indIndexScaledOffsetI2LN(iRegN reg, iRegI ireg, immIScale scale, immLAddSub off)
+operand indIndexScaledOffsetI2LN(iRegN reg, iRegI ireg, immIScale scale, immLU12 off)
 %{
   predicate(Universe::narrow_oop_shift() == 0);
   constraint(ALLOC_IN_RC(ptr_reg));
@@ -4333,7 +4351,7 @@
   %}
 %}

-operand indOffIN(iRegN reg, immIAddSub off)
+operand indOffIN(iRegN reg, immIOffset off)
 %{
   predicate(Universe::narrow_oop_shift() == 0);
   constraint(ALLOC_IN_RC(ptr_reg));
@@ -4348,7 +4366,7 @@
   %}
 %}

-operand indOffLN(iRegN reg, immLAddSub off)
+operand indOffLN(iRegN reg, immLoffset off)
 %{
   predicate(Universe::narrow_oop_shift() == 0);
   constraint(ALLOC_IN_RC(ptr_reg));