[aarch64-port-dev ] storeImmNKlass is broken

[Andrew Haley]

This pattern:

// Store Compressed Klass Pointer Constant
instruct storeImmNKlass(immN src, memory mem)
%{
  match(Set mem (StoreNKlass mem src));

  ins_cost(MEMORY_REF_COST_LOW);
  format %{ "strw  $src, $mem\t# compressed klass ptr" %}

  ins_encode(aarch64_enc_strw_immnk(src, mem));

  ins_pipe(pipe_class_memory);
%}

breaks, because it results in

  0x00007fffed1997c0: ldr	xscratch2, 0x00007fffed1996a8
                                                ;   {section_word}
                                                ; implicit exception: dispatches to 0x00007fffed19986c
  0x00007fffed1997c4: cbnz	xscratch2, 0x00007fffed199838
   ...
  0x00007fffed199838: str	wscratch2, [x10,#12]

Note that the implicit exception is attached to the insn at
0x00007fffed1997c0: the cpool load.  The place where a
NullPointerException might actually happen is insn 0x00007fffed199838,
but it has no implicit exception marker.  If x10 is null, the VM
aborts.

I don't think there's any point having this pattern, or anything like
it.  It doesn't improve code quality.  Here's what I propose to do.

Andrew.


# HG changeset patch
# User aph
# Date 1384265283 0
# Node ID 31a8727c3583744b28c9ac7da70bca4332b75967
# Parent  19d511645e2ed8b664fca4cecabf6eb3c2b75fb5
Delete patterns that sture klass/oop constants into memory

diff -r 19d511645e2e -r 31a8727c3583 src/cpu/aarch64/vm/aarch64.ad
--- a/src/cpu/aarch64/vm/aarch64.ad	Sat Nov 09 11:20:38 2013 +0000
+++ b/src/cpu/aarch64/vm/aarch64.ad	Tue Nov 12 14:08:03 2013 +0000
@@ -2037,28 +2037,6 @@
                as_Register($mem$$base), $mem$$index, $mem$$scale, $mem$$disp);
   %}

-  enc_class aarch64_enc_strw_immn(immN src, memory mem) %{
-    MacroAssembler _masm(&cbuf);
-    address con = (address)$src$$constant;
-    // need to do this the hard way until we can manage relocs
-    // for 32 bit constants
-    __ movoop(rscratch2, (jobject)con);
-    if (con) __ encode_heap_oop_not_null(rscratch2);
-    loadStore(_masm, &MacroAssembler::strw, rscratch2, $mem->opcode(),
-               as_Register($mem$$base), $mem$$index, $mem$$scale, $mem$$disp);
-  %}
-
-  enc_class aarch64_enc_strw_immnk(immN src, memory mem) %{
-    MacroAssembler _masm(&cbuf);
-    address con = (address)$src$$constant;
-    // need to do this the hard way until we can manage relocs
-    // for 32 bit constants
-    __ movoop(rscratch2, (jobject)con);
-    __ encode_klass_not_null(rscratch2);
-    loadStore(_masm, &MacroAssembler::strw, rscratch2, $mem->opcode(),
-               as_Register($mem$$base), $mem$$index, $mem$$scale, $mem$$disp);
-  %}
-
   // END Non-volatile memory access

   // this encoding writes the address of the first instruction in the
@@ -2326,34 +2304,6 @@
 		 rscratch1, stlr);
   %}

-  enc_class aarch64_enc_stlrw_immn(immN src, memory mem) %{
-    {
-      MacroAssembler _masm(&cbuf);
-      address con = (address)$src$$constant;
-      // need to do this the hard way until we can manage relocs
-      // for 32 bit constants
-      __ movoop(rscratch2, (jobject)con);
-      if (con) __ encode_heap_oop_not_null(rscratch2);
-    }
-    MOV_VOLATILE(rscratch2, $mem$$base, $mem$$index, $mem$$scale, $mem$$disp,
-		 rscratch1, stlrw);
-  %}
-
-  enc_class aarch64_enc_stlrw_immnk(immN src, memory mem) %{
-    {
-      MacroAssembler _masm(&cbuf);
-      address con = (address)$src$$constant;
-      // need to do this the hard way until we can manage relocs
-      // for 32 bit constants
-      __ movoop(rscratch2, (jobject)con);
-      // Either it is a heap oop or a klass pointer? It can't be both?
-      // __ encode_heap_oop_not_null(rscratch2);
-      if (con) __ encode_klass_not_null(rscratch2);
-    }
-    MOV_VOLATILE(rscratch2, $mem$$base, $mem$$index, $mem$$scale, $mem$$disp,
-		 rscratch1, stlrw);
-  %}
-
   // synchronized read/update encodings

   enc_class aarch64_enc_ldaxr(iRegL dst, memory mem) %{
@@ -5363,20 +5313,6 @@
   ins_pipe(pipe_class_memory);
 %}

-instruct storeImmN(immN src, memory mem)
-%{
-  match(Set mem (StoreN mem src));
-  predicate(!((MemNode*)n)->is_volatile());
-
-  ins_cost(MEMORY_REF_COST);
-  format %{ "strw  $src, $mem\t# compressed ptr " %}
-
-  ins_encode(aarch64_enc_strw_immn(src, mem));
-
-  ins_pipe(pipe_class_memory);
-%}
-
-
 // Store Float
 instruct storeF(vRegF src, memory mem)
 %{
@@ -5421,19 +5357,6 @@
   ins_pipe(pipe_class_memory);
 %}

-// Store Compressed Klass Pointer Constant
-instruct storeImmNKlass(immN src, memory mem)
-%{
-  match(Set mem (StoreNKlass mem src));
-
-  ins_cost(MEMORY_REF_COST_LOW);
-  format %{ "strw  $src, $mem\t# compressed klass ptr" %}
-
-  ins_encode(aarch64_enc_strw_immnk(src, mem));
-
-  ins_pipe(pipe_class_memory);
-%}
-
 // TODO
 // implement storeImmD0 and storeDImmPacked