6965570: assert(!needs_patching && x->is_loaded(),"how do we know it's...

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

@@ -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;
     }
   }

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);
         }

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());

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;

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();
     }
   }