From 49a0bf9fb1524283c7aab7bdbda6ddbfcab8ce6f Mon Sep 17 00:00:00 2001 From: never Date: Sun, 6 Mar 2011 22:09:23 -0800 Subject: [PATCH] 6965570: assert(!needs_patching && x->is_loaded(),"how do we know it's volatile if it's not loaded") Reviewed-by: iveresov --- src/share/vm/c1/c1_Canonicalizer.cpp | 2 +- src/share/vm/c1/c1_GraphBuilder.cpp | 26 ++++++++++-------------- src/share/vm/c1/c1_Instruction.hpp | 30 +++++++++++----------------- src/share/vm/c1/c1_LIRGenerator.cpp | 8 ++------ src/share/vm/c1/c1_ValueMap.hpp | 6 ++++-- 5 files changed, 30 insertions(+), 42 deletions(-) diff --git a/src/share/vm/c1/c1_Canonicalizer.cpp b/src/share/vm/c1/c1_Canonicalizer.cpp index a2d400940..3291fe18f 100644 --- a/src/share/vm/c1/c1_Canonicalizer.cpp +++ b/src/share/vm/c1/c1_Canonicalizer.cpp @@ -209,7 +209,7 @@ void Canonicalizer::do_StoreField (StoreField* x) { // limit this optimization to current block if (value != NULL && in_current_block(conv)) { set_canonical(new StoreField(x->obj(), x->offset(), x->field(), value, x->is_static(), - x->state_before(), x->is_loaded(), x->is_initialized())); + x->state_before(), x->needs_patching())); return; } } diff --git a/src/share/vm/c1/c1_GraphBuilder.cpp b/src/share/vm/c1/c1_GraphBuilder.cpp index c31c407c3..f6b019296 100644 --- a/src/share/vm/c1/c1_GraphBuilder.cpp +++ b/src/share/vm/c1/c1_GraphBuilder.cpp @@ -1456,12 +1456,12 @@ void GraphBuilder::access_field(Bytecodes::Code code) { BasicType field_type = field->type()->basic_type(); ValueType* type = as_ValueType(field_type); // call will_link again to determine if the field is valid. - const bool is_loaded = holder->is_loaded() && - field->will_link(method()->holder(), code); - const bool is_initialized = is_loaded && holder->is_initialized(); + const bool needs_patching = !holder->is_loaded() || + !field->will_link(method()->holder(), code) || + PatchALot; ValueStack* state_before = NULL; - if (!is_initialized || PatchALot) { + if (!holder->is_initialized() || needs_patching) { // save state before instruction for debug info when // deoptimization happens during patching state_before = copy_state_before(); @@ -1469,10 +1469,6 @@ void GraphBuilder::access_field(Bytecodes::Code code) { Value obj = NULL; if (code == Bytecodes::_getstatic || code == Bytecodes::_putstatic) { - // commoning of class constants should only occur if the class is - // fully initialized and resolved in this constant pool. The will_link test - // above essentially checks if this class is resolved in this constant pool - // so, the is_initialized flag should be suffiect. if (state_before != NULL) { // build a patching constant obj = new Constant(new ClassConstant(holder), state_before); @@ -1482,7 +1478,7 @@ void GraphBuilder::access_field(Bytecodes::Code code) { } - const int offset = is_loaded ? field->offset() : -1; + const int offset = !needs_patching ? field->offset() : -1; switch (code) { case Bytecodes::_getstatic: { // check for compile-time constants, i.e., initialized static final fields @@ -1509,7 +1505,7 @@ void GraphBuilder::access_field(Bytecodes::Code code) { state_before = copy_state_for_exception(); } push(type, append(new LoadField(append(obj), offset, field, true, - state_before, is_loaded, is_initialized))); + state_before, needs_patching))); } break; } @@ -1518,7 +1514,7 @@ void GraphBuilder::access_field(Bytecodes::Code code) { if (state_before == NULL) { state_before = copy_state_for_exception(); } - append(new StoreField(append(obj), offset, field, val, true, state_before, is_loaded, is_initialized)); + append(new StoreField(append(obj), offset, field, val, true, state_before, needs_patching)); } break; case Bytecodes::_getfield : @@ -1526,8 +1522,8 @@ void GraphBuilder::access_field(Bytecodes::Code code) { if (state_before == NULL) { state_before = copy_state_for_exception(); } - LoadField* load = new LoadField(apop(), offset, field, false, state_before, is_loaded, true); - Value replacement = is_loaded ? _memory->load(load) : load; + LoadField* load = new LoadField(apop(), offset, field, false, state_before, needs_patching); + Value replacement = !needs_patching ? _memory->load(load) : load; if (replacement != load) { assert(replacement->is_linked() || !replacement->can_be_linked(), "should already by linked"); push(type, replacement); @@ -1542,8 +1538,8 @@ void GraphBuilder::access_field(Bytecodes::Code code) { if (state_before == NULL) { state_before = copy_state_for_exception(); } - StoreField* store = new StoreField(apop(), offset, field, val, false, state_before, is_loaded, true); - if (is_loaded) store = _memory->store(store); + StoreField* store = new StoreField(apop(), offset, field, val, false, state_before, needs_patching); + if (!needs_patching) store = _memory->store(store); if (store != NULL) { append(store); } diff --git a/src/share/vm/c1/c1_Instruction.hpp b/src/share/vm/c1/c1_Instruction.hpp index 3a5eecc2d..d997c572c 100644 --- a/src/share/vm/c1/c1_Instruction.hpp +++ b/src/share/vm/c1/c1_Instruction.hpp @@ -323,8 +323,6 @@ class Instruction: public CompilationResourceObj { CanTrapFlag, DirectCompareFlag, IsEliminatedFlag, - IsInitializedFlag, - IsLoadedFlag, IsSafepointFlag, IsStaticFlag, IsStrictfpFlag, @@ -693,7 +691,7 @@ BASE(AccessField, Instruction) public: // creation AccessField(Value obj, int offset, ciField* field, bool is_static, - ValueStack* state_before, bool is_loaded, bool is_initialized) + ValueStack* state_before, bool needs_patching) : Instruction(as_ValueType(field->type()->basic_type()), state_before) , _obj(obj) , _offset(offset) @@ -701,16 +699,9 @@ BASE(AccessField, Instruction) , _explicit_null_check(NULL) { set_needs_null_check(!is_static); - set_flag(IsLoadedFlag, is_loaded); - set_flag(IsInitializedFlag, is_initialized); set_flag(IsStaticFlag, is_static); + set_flag(NeedsPatchingFlag, needs_patching); ASSERT_VALUES - if (!is_loaded || (PatchALot && !field->is_volatile())) { - // need to patch if the holder wasn't loaded or we're testing - // using PatchALot. Don't allow PatchALot for fields which are - // known to be volatile they aren't patchable. - set_flag(NeedsPatchingFlag, true); - } // pin of all instructions with memory access pin(); } @@ -721,11 +712,14 @@ BASE(AccessField, Instruction) ciField* field() const { return _field; } BasicType field_type() const { return _field->type()->basic_type(); } bool is_static() const { return check_flag(IsStaticFlag); } - bool is_loaded() const { return check_flag(IsLoadedFlag); } - bool is_initialized() const { return check_flag(IsInitializedFlag); } NullCheck* explicit_null_check() const { return _explicit_null_check; } bool needs_patching() const { return check_flag(NeedsPatchingFlag); } + // Unresolved getstatic and putstatic can cause initialization. + // Technically it occurs at the Constant that materializes the base + // of the static fields but it's simpler to model it here. + bool is_init_point() const { return is_static() && (needs_patching() || !_field->holder()->is_initialized()); } + // manipulation // Under certain circumstances, if a previous NullCheck instruction @@ -745,15 +739,15 @@ LEAF(LoadField, AccessField) public: // creation LoadField(Value obj, int offset, ciField* field, bool is_static, - ValueStack* state_before, bool is_loaded, bool is_initialized) - : AccessField(obj, offset, field, is_static, state_before, is_loaded, is_initialized) + ValueStack* state_before, bool needs_patching) + : AccessField(obj, offset, field, is_static, state_before, needs_patching) {} ciType* declared_type() const; ciType* exact_type() const; // generic - HASHING2(LoadField, is_loaded() && !field()->is_volatile(), obj()->subst(), offset()) // cannot be eliminated if not yet loaded or if volatile + HASHING2(LoadField, !needs_patching() && !field()->is_volatile(), obj()->subst(), offset()) // cannot be eliminated if needs patching or if volatile }; @@ -764,8 +758,8 @@ LEAF(StoreField, AccessField) public: // creation StoreField(Value obj, int offset, ciField* field, Value value, bool is_static, - ValueStack* state_before, bool is_loaded, bool is_initialized) - : AccessField(obj, offset, field, is_static, state_before, is_loaded, is_initialized) + ValueStack* state_before, bool needs_patching) + : AccessField(obj, offset, field, is_static, state_before, needs_patching) , _value(value) { set_flag(NeedsWriteBarrierFlag, as_ValueType(field_type())->is_object()); diff --git a/src/share/vm/c1/c1_LIRGenerator.cpp b/src/share/vm/c1/c1_LIRGenerator.cpp index 3faff5367..a23a79410 100644 --- a/src/share/vm/c1/c1_LIRGenerator.cpp +++ b/src/share/vm/c1/c1_LIRGenerator.cpp @@ -1559,9 +1559,7 @@ void LIRGenerator::do_StoreField(StoreField* x) { (info ? new CodeEmitInfo(info) : NULL)); } - if (is_volatile) { - assert(!needs_patching && x->is_loaded(), - "how do we know it's volatile if it's not loaded"); + if (is_volatile && !needs_patching) { volatile_field_store(value.result(), address, info); } else { LIR_PatchCode patch_code = needs_patching ? lir_patch_normal : lir_patch_none; @@ -1627,9 +1625,7 @@ void LIRGenerator::do_LoadField(LoadField* x) { address = generate_address(object.result(), x->offset(), field_type); } - if (is_volatile) { - assert(!needs_patching && x->is_loaded(), - "how do we know it's volatile if it's not loaded"); + if (is_volatile && !needs_patching) { volatile_field_load(address, reg, info); } else { LIR_PatchCode patch_code = needs_patching ? lir_patch_normal : lir_patch_none; diff --git a/src/share/vm/c1/c1_ValueMap.hpp b/src/share/vm/c1/c1_ValueMap.hpp index 7dcae91f3..248d17496 100644 --- a/src/share/vm/c1/c1_ValueMap.hpp +++ b/src/share/vm/c1/c1_ValueMap.hpp @@ -141,7 +141,8 @@ class ValueNumberingVisitor: public InstructionVisitor { // visitor functions void do_StoreField (StoreField* x) { - if (!x->is_initialized()) { + if (x->is_init_point()) { + // putstatic is an initialization point so treat it as a wide kill kill_memory(); } else { kill_field(x->field()); @@ -159,7 +160,8 @@ class ValueNumberingVisitor: public InstructionVisitor { void do_Local (Local* x) { /* nothing to do */ } void do_Constant (Constant* x) { /* nothing to do */ } void do_LoadField (LoadField* x) { - if (!x->is_initialized()) { + if (x->is_init_point()) { + // getstatic is an initialization point so treat it as a wide kill kill_memory(); } } -- GitLab