Skip to content
Permalink
Browse files

8245953: [lworld] Remove array storage property bits clearing from JI…

…T code
  • Loading branch information
Tobias Hartmann
Tobias Hartmann committed May 27, 2020
1 parent 8422fb4 commit 27e8b6e06f28e6096072d23175af630915af7d22
@@ -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());
@@ -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)
@@ -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-----------------------------------
@@ -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);

@@ -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);
@@ -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;
@@ -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------------------------------------------
@@ -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--------------------------------------
@@ -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);
@@ -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;
@@ -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;
}
@@ -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;
}
}
@@ -219,7 +219,6 @@
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;

0 comments on commit 27e8b6e

Please sign in to comment.
You can’t perform that action at this time.