8245953: [lworld] Remove array storage property bits clearing from JIT code

Author: TobiHartmann

src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp

@@ -1400,7 +1400,6 @@ void LIR_Assembler::mem2reg(LIR_Opr src, LIR_Opr dest, BasicType type, LIR_Patch
       __ verify_oop(dest->as_register());
     }
   } else if (type == T_ADDRESS && addr->disp() == oopDesc::klass_offset_in_bytes()) {
-    // TODO remove clear_prop_bits bits stuff once the runtime does not set it anymore
 #ifdef _LP64
     if (UseCompressedClassPointers) {
       __ decode_klass_not_null(dest->as_register());

src/hotspot/cpu/x86/x86_64.ad

@@ -11760,21 +11760,6 @@ instruct testI_mem_imm(rFlagsReg cr, memory mem, immI con, immI0 zero)
   ins_pipe(ialu_mem_imm);
 %}
 
-// Clear array property bits
-instruct clear_property_bits(rRegN dst, memory mem, immU31 mask, rFlagsReg cr)
-%{
-  match(Set dst (CastI2N (AndI (CastN2I (LoadNKlass mem)) mask)));
-  effect(KILL cr);
-
-  format %{ "movl    $dst, $mem\t# clear property bits\n\t"
-            "andl    $dst, $mask" %}
-  ins_encode %{
-    __ movl($dst$$Register, $mem$$Address);
-    __ andl($dst$$Register, $mask$$constant);
-  %}
-  ins_pipe(ialu_reg_mem);
-%}
-
 // Unsigned compare Instructions; really, same as signed except they
 // produce an rFlagsRegU instead of rFlagsReg.
 instruct compU_rReg(rFlagsRegU cr, rRegI op1, rRegI op2)

src/hotspot/share/opto/graphKit.cpp

@@ -1177,13 +1177,12 @@ Node* GraphKit::ConvL2I(Node* offset) {
 }
 
 //-------------------------load_object_klass-----------------------------------
-Node* GraphKit::load_object_klass(Node* obj, bool clear_prop_bits) {
+Node* GraphKit::load_object_klass(Node* obj) {
   // Special-case a fresh allocation to avoid building nodes:
   Node* akls = AllocateNode::Ideal_klass(obj, &_gvn);
   if (akls != NULL)  return akls;
   Node* k_adr = basic_plus_adr(obj, oopDesc::klass_offset_in_bytes());
-  // TODO remove clear_prop_bits bits stuff once the runtime does not set it anymore
-  return _gvn.transform(LoadKlassNode::make(_gvn, NULL, immutable_memory(), k_adr, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT, clear_prop_bits));
+  return _gvn.transform(LoadKlassNode::make(_gvn, NULL, immutable_memory(), k_adr, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT));
 }
 
 //-------------------------load_array_length-----------------------------------

src/hotspot/share/opto/graphKit.hpp

@@ -342,7 +342,7 @@ class GraphKit : public Phase {
   Node* ConvI2UL(Node* offset);
   Node* ConvL2I(Node* offset);
   // Find out the klass of an object.
-  Node* load_object_klass(Node* object, bool clear_prop_bits = true);
+  Node* load_object_klass(Node* object);
   // Find out the length of an array.
   Node* load_array_length(Node* array);
 

src/hotspot/share/opto/macro.cpp

@@ -2864,7 +2864,7 @@ void PhaseMacroExpand::expand_subtypecheck_node(SubTypeCheckNode *check) {
       subklass = obj_or_subklass;
     } else {
       Node* k_adr = basic_plus_adr(obj_or_subklass, oopDesc::klass_offset_in_bytes());
-      subklass = _igvn.transform(LoadKlassNode::make(_igvn, NULL, C->immutable_memory(), k_adr, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT, true));
+      subklass = _igvn.transform(LoadKlassNode::make(_igvn, NULL, C->immutable_memory(), k_adr, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT));
     }
 
     Node* not_subtype_ctrl = Phase::gen_subtype_check(subklass, superklass, &ctrl, NULL, _igvn);

src/hotspot/share/opto/macroArrayCopy.cpp

@@ -203,7 +203,7 @@ Node* PhaseMacroExpand::generate_array_guard(Node** ctrl, Node* mem, Node* obj_o
   Node* kls = NULL;
   if (_igvn.type(obj_or_klass)->isa_oopptr()) {
     Node* k_adr = basic_plus_adr(obj_or_klass, oopDesc::klass_offset_in_bytes());
-    kls = transform_later(LoadKlassNode::make(_igvn, NULL, C->immutable_memory(), k_adr, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT, /* clear_prop_bits = */ true));
+    kls = transform_later(LoadKlassNode::make(_igvn, NULL, C->immutable_memory(), k_adr, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT));
   } else {
     assert(_igvn.type(obj_or_klass)->isa_klassptr(), "what else?");
     kls = obj_or_klass;

src/hotspot/share/opto/memnode.cpp

@@ -2184,19 +2184,19 @@ const Type* LoadSNode::Value(PhaseGVN* phase) const {
 //----------------------------LoadKlassNode::make------------------------------
 // Polymorphic factory method:
 Node* LoadKlassNode::make(PhaseGVN& gvn, Node* ctl, Node* mem, Node* adr, const TypePtr* at,
-                          const TypeKlassPtr* tk, bool clear_prop_bits) {
+                          const TypeKlassPtr* tk) {
   // sanity check the alias category against the created node type
   const TypePtr *adr_type = adr->bottom_type()->isa_ptr();
   assert(adr_type != NULL, "expecting TypeKlassPtr");
 #ifdef _LP64
   if (adr_type->is_ptr_to_narrowklass()) {
     assert(UseCompressedClassPointers, "no compressed klasses");
-    Node* load_klass = gvn.transform(new LoadNKlassNode(ctl, mem, adr, at, tk->make_narrowklass(), MemNode::unordered, clear_prop_bits));
+    Node* load_klass = gvn.transform(new LoadNKlassNode(ctl, mem, adr, at, tk->make_narrowklass(), MemNode::unordered));
     return new DecodeNKlassNode(load_klass, load_klass->bottom_type()->make_ptr());
   }
 #endif
   assert(!adr_type->is_ptr_to_narrowklass() && !adr_type->is_ptr_to_narrowoop(), "should have got back a narrow oop");
-  return new LoadKlassNode(ctl, mem, adr, at, tk, MemNode::unordered, clear_prop_bits);
+  return new LoadKlassNode(ctl, mem, adr, at, tk, MemNode::unordered);
 }
 
 //------------------------------Value------------------------------------------

src/hotspot/share/opto/memnode.hpp

@@ -515,44 +515,29 @@ class LoadNNode : public LoadNode {
 //------------------------------LoadKlassNode----------------------------------
 // Load a Klass from an object
 class LoadKlassNode : public LoadPNode {
-private:
-  bool _clear_prop_bits; // Clear the ArrayStorageProperties bits
 protected:
   // In most cases, LoadKlassNode does not have the control input set. If the control
   // input is set, it must not be removed (by LoadNode::Ideal()).
   virtual bool can_remove_control() const;
 public:
-  LoadKlassNode(Node *c, Node *mem, Node *adr, const TypePtr *at, const TypeKlassPtr *tk, MemOrd mo, bool clear_prop_bits)
-    : LoadPNode(c, mem, adr, at, tk, mo), _clear_prop_bits(clear_prop_bits) {}
-  virtual uint hash() const { return LoadPNode::hash() + _clear_prop_bits; }
-  virtual bool cmp(const Node &n) const {
-    return (_clear_prop_bits == ((LoadKlassNode&)n)._clear_prop_bits) && LoadPNode::cmp(n);
-  }
-  virtual uint size_of() const { return sizeof(*this); }
+  LoadKlassNode(Node *c, Node *mem, Node *adr, const TypePtr *at, const TypeKlassPtr *tk, MemOrd mo)
+    : LoadPNode(c, mem, adr, at, tk, mo) {}
   virtual int Opcode() const;
   virtual const Type* Value(PhaseGVN* phase) const;
   virtual Node* Identity(PhaseGVN* phase);
   virtual bool depends_only_on_test() const { return true; }
-  bool clear_prop_bits() const { return _clear_prop_bits; }
 
   // Polymorphic factory method:
   static Node* make(PhaseGVN& gvn, Node* ctl, Node* mem, Node* adr, const TypePtr* at,
-                    const TypeKlassPtr* tk = TypeKlassPtr::OBJECT, bool clear_prop_bits = false);
+                    const TypeKlassPtr* tk = TypeKlassPtr::OBJECT);
 };
 
 //------------------------------LoadNKlassNode---------------------------------
 // Load a narrow Klass from an object.
 class LoadNKlassNode : public LoadNNode {
-private:
-  bool _clear_prop_bits; // Clear the ArrayStorageProperties bits
 public:
-  LoadNKlassNode(Node *c, Node *mem, Node *adr, const TypePtr *at, const TypeNarrowKlass *tk, MemOrd mo, bool clear_prop_bits)
-    : LoadNNode(c, mem, adr, at, tk, mo), _clear_prop_bits(clear_prop_bits) {}
-  virtual uint hash() const { return LoadNNode::hash() + _clear_prop_bits; }
-  virtual bool cmp(const Node &n) const {
-    return (_clear_prop_bits == ((LoadNKlassNode&)n)._clear_prop_bits) && LoadNNode::cmp(n);
-  }
-  virtual uint size_of() const { return sizeof(*this); }
+  LoadNKlassNode(Node *c, Node *mem, Node *adr, const TypePtr *at, const TypeNarrowKlass *tk, MemOrd mo)
+    : LoadNNode(c, mem, adr, at, tk, mo) {}
   virtual int Opcode() const;
   virtual uint ideal_reg() const { return Op_RegN; }
   virtual int store_Opcode() const { return Op_StoreNKlass; }
@@ -561,7 +546,6 @@ class LoadNKlassNode : public LoadNNode {
   virtual const Type* Value(PhaseGVN* phase) const;
   virtual Node* Identity(PhaseGVN* phase);
   virtual bool depends_only_on_test() const { return true; }
-  bool clear_prop_bits() const { return _clear_prop_bits; }
 };
 
 //------------------------------StoreNode--------------------------------------

src/hotspot/share/opto/parse2.cpp

@@ -2162,10 +2162,9 @@ void Parse::do_acmp(BoolTest::mask btest, Node* a, Node* b) {
     return;
   }
 
-  // Check if both operands are of the same class. We don't need to clear the array property
-  // bits in the klass pointer for the cmp because we know that the first operand is a value type.
-  Node* kls_a = load_object_klass(not_null_a, /* clear_prop_bits = */ false);
-  Node* kls_b = load_object_klass(not_null_b, /* clear_prop_bits = */ false);
+  // Check if both operands are of the same class.
+  Node* kls_a = load_object_klass(not_null_a);
+  Node* kls_b = load_object_klass(not_null_b);
   Node* kls_cmp = CmpP(kls_a, kls_b);
   Node* kls_bol = _gvn.transform(new BoolNode(kls_cmp, BoolTest::ne));
   IfNode* kls_iff = create_and_map_if(control(), kls_bol, PROB_FAIR, COUNT_UNKNOWN);

test/hotspot/jtreg/compiler/valhalla/valuetypes/TestArrays.java

@@ -787,7 +787,7 @@ public void test33_verifier(boolean warmup) {
         Object[] result = test33(va);
         for (int i = 0; i < len; ++i) {
             Asserts.assertEQ(((MyValue1)result[i]).hash(), ((MyValue1)va[i]).hash());
-            // Check that array has correct storage properties (null-ok)
+            // Check that array has correct properties (null-ok)
             result[i] = null;
         }
     }
@@ -822,7 +822,7 @@ public void test34_verifier(boolean warmup) {
         for (int i = 0; i < 10; i++) { // make sure we do deopt
             Object[] result = test34(true);
             verify(test34_orig, result);
-            // Check that array has correct storage properties (null-free)
+            // Check that array has correct properties (null-free)
             try {
                 result[0] = null;
                 throw new RuntimeException("Should throw NullPointerException");
@@ -833,7 +833,7 @@ public void test34_verifier(boolean warmup) {
         if (compile_and_run_again_if_deoptimized(warmup, "TestArrays::test34")) {
             Object[] result = test34(true);
             verify(test34_orig, result);
-            // Check that array has correct storage properties (null-free)
+            // Check that array has correct properties (null-free)
             try {
                 result[0] = null;
                 throw new RuntimeException("Should throw NullPointerException");
@@ -1750,7 +1750,7 @@ public void test75_verifier(boolean warmup) {
 
         for (int i = 0; i < va.length; ++i) {
             Asserts.assertEQ(oa[i], result[i]);
-            // Check that array has correct storage properties (null-ok)
+            // Check that array has correct properties (null-ok)
             result[i] = null;
         }
     }
@@ -1788,7 +1788,7 @@ public void test76_verifier(boolean warmup) {
         test76_helper(42, va, oa);
         Object[] result = test76(va, oa);
         verify(verif, result);
-        // Check that array has correct storage properties (null-free)
+        // Check that array has correct properties (null-free)
         if (len > 0) {
             try {
                 result[0] = null;

test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java

@@ -2361,10 +2361,7 @@ public void test88_verifier(boolean warmup) {
                                         Asserts.assertTrue(Arrays.equals(src2, dst2)); });
     }
 
-    // Verify that the storage property bits in the klass pointer are
-    // not cleared if we are comparing to a klass that can't be a inline
-    // type array klass anyway.
-    @Test(failOn = STORAGE_PROPERTY_CLEARING)
+    @Test
     public boolean test89(Object obj) {
         return obj.getClass() == Integer.class;
     }
@@ -2375,8 +2372,7 @@ public void test89_verifier(boolean warmup) {
         Asserts.assertFalse(test89(new Object()));
     }
 
-    // Same as test89 but with a cast
-    @Test(failOn = STORAGE_PROPERTY_CLEARING)
+    @Test
     public Integer test90(Object obj) {
         return (Integer)obj;
     }
@@ -2392,9 +2388,7 @@ public void test90_verifier(boolean warmup) {
         }
     }
 
-    // Same as test89 but bit clearing can not be removed because
-    // we are comparing to a inline type array klass.
-    @Test(match = {STORAGE_PROPERTY_CLEARING}, matchCount = { 1 })
+    @Test
     public boolean test91(Object obj) {
         return obj.getClass() == MyValue2[].class;
     }
@@ -2498,10 +2492,8 @@ public void test94_verifier(boolean warmup) {
         Asserts.assertEquals(result, 0x42 * 2);
     }
 
-    // Test that no code for clearing the array klass property bits is emitted for acmp
-    // because when loading the klass, we already know that the operand is a value type.
     @Warmup(10000)
-    @Test(failOn = STORAGE_PROPERTY_CLEARING)
+    @Test
     public boolean test95(Object o1, Object o2) {
         return o1 == o2;
     }
@@ -2516,9 +2508,8 @@ public void test95_verifier(boolean warmup) {
         Asserts.assertFalse(test95(o1, o2));
     }
 
-    // Same as test95 but operands are never null
     @Warmup(10000)
-    @Test(failOn = STORAGE_PROPERTY_CLEARING)
+    @Test
     public boolean test96(Object o1, Object o2) {
         return o1 == o2;
     }

test/hotspot/jtreg/compiler/valhalla/valuetypes/TestNullableArrays.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1933,7 +1933,7 @@ public void test74_verifier(boolean warmup) {
 
         for (int i = 0; i < va.length; ++i) {
             Asserts.assertEQ(oa[i], result[i]);
-            // Check that array has correct storage properties (null-ok)
+            // Check that array has correct properties (null-ok)
             result[i] = null;
         }
     }
@@ -1972,7 +1972,7 @@ public void test75_verifier(boolean warmup) {
         Object[] result = test75(va, oa);
         verify(verif, result);
         if (len > 0) {
-            // Check that array has correct storage properties (null-ok)
+            // Check that array has correct properties (null-ok)
             result[0] = null;
         }
     }

test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java

@@ -219,7 +219,6 @@ public abstract class ValueTypeTest {
     protected static final String LOAD_UNKNOWN_VALUE = "(.*call_leaf,runtime  load_unknown_value.*" + END;
     protected static final String STORE_UNKNOWN_VALUE = "(.*call_leaf,runtime  store_unknown_value.*" + END;
     protected static final String VALUE_ARRAY_NULL_GUARD = "(.*call,static  wrapper for: uncommon_trap.*reason='null_check' action='none'.*" + END;
-    protected static final String STORAGE_PROPERTY_CLEARING = "(.*((int:536870911)|(salq.*3\\R.*sarq.*3)).*" + END;
     protected static final String CLASS_CHECK_TRAP = START + "CallStaticJava" + MID + "uncommon_trap.*class_check" + END;
     protected static final String NULL_CHECK_TRAP = START + "CallStaticJava" + MID + "uncommon_trap.*null_check" + END;
     protected static final String RANGE_CHECK_TRAP = START + "CallStaticJava" + MID + "uncommon_trap.*range_check" + END;