You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by mf...@apache.org on 2007/08/15 09:38:47 UTC

svn commit: r566044 - in /harmony/enhanced/drlvm/trunk/vm/jitrino/src: jet/cg.h jet/cg_fld_arr.cpp jet/cg_instr.cpp translator/java/JavaByteCodeTranslator.cpp

Author: mfursov
Date: Wed Aug 15 00:38:46 2007
New Revision: 566044

URL: http://svn.apache.org/viewvc?view=rev&rev=566044
Log:
Fix for HARMONY-4452 "[drlvm][jit] write barrier in OPT does not catch all reference fields updates"

Modified:
    harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h
    harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp
    harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp
    harmony/enhanced/drlvm/trunk/vm/jitrino/src/translator/java/JavaByteCodeTranslator.cpp

Modified: harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h?view=diff&rev=566044&r1=566043&r2=566044
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h (original)
+++ harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h Wed Aug 15 00:38:46 2007
@@ -1109,6 +1109,12 @@
     void do_field_op(const FieldOpInfo& fieldOp);
 
     /**
+    * @brief Returns mem-opnd that is address of the given field. Used by do_field_op
+    * Invokes gen_check_null() for GETFIELD and PUTFIELD.
+    */
+    Opnd get_field_addr(const FieldOpInfo& fieldOp, jtype jt);
+
+    /**
      * PMF instance to get arguments from.
      */
     PMF *               m_pmf;

Modified: harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp?view=diff&rev=566044&r1=566043&r2=566044
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp (original)
+++ harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp Wed Aug 15 00:38:46 2007
@@ -235,36 +235,22 @@
     do_field_op(fieldOp);
 }
 
-
-void CodeGen::do_field_op(const FieldOpInfo& fieldOp)
-{
-    // Presumption: we dont have compressed refs on IA32 and all other 
-    // (64bits) platforms have compressed refs. 
-    // is_ia32() check added below so on IA32 it becomes 'false' during the 
-    // compilation, without access to g_refs_squeeze in runtime.
-    assert(is_ia32() || g_refs_squeeze);
-
-
-    jtype jt = to_jtype(class_get_cp_field_type(fieldOp.enclClass, fieldOp.cpIndex));
-    
-    const char* fieldDescName = const_pool_get_field_descriptor(fieldOp.enclClass, fieldOp.cpIndex);
-    bool fieldIsMagic = is_magic_class(fieldDescName);
-    if (fieldIsMagic) {
-        jt = iplatf;
-    }
-
+Opnd CodeGen::get_field_addr(const FieldOpInfo& fieldOp, jtype jt) {
     Opnd where;
     if (!fieldOp.isStatic()) { //generate check null
         unsigned ref_depth = fieldOp.isPut() ? (is_wide(jt) ? 2 : 1) : 0;
-        Val& ref = vstack(ref_depth, true);
         gen_check_null(ref_depth);
         if (fieldOp.fld) { //field is resolved -> offset is available
             unsigned fld_offset = field_get_offset(fieldOp.fld);
+            Val& ref = vstack(ref_depth, true);
             where = Opnd(jt, ref.reg(), fld_offset);
         }  else { //field is not resolved -> generate code to request offset
             static const CallSig cs_get_offset(CCONV_STDCALL, iplatf, i32, i32);
             gen_call_vm(cs_get_offset, rt_helper_field_get_offset_withresolve, 0, fieldOp.enclClass, fieldOp.cpIndex, fieldOp.isPut());
             runlock(cs_get_offset);
+            rlock(gr_ret);
+            Val& ref = vstack(ref_depth, true);
+            runlock(gr_ret);
             alu(alu_add, gr_ret, ref.as_opnd());
             where = Opnd(jt, gr_ret, 0);
         }
@@ -279,9 +265,25 @@
             where = Opnd(jt, gr_ret, 0);
         }
     }
-    rlock(where);
+    return where;
+}
+
+void CodeGen::do_field_op(const FieldOpInfo& fieldOp)
+{
+    // Presumption: we dont have compressed refs on IA32 and all other 
+    // (64bits) platforms have compressed refs. 
+    // is_ia32() check added below so on IA32 it becomes 'false' during the 
+    // compilation, without access to g_refs_squeeze in runtime.
+    assert(is_ia32() || g_refs_squeeze);
 
-    // notify all listeners
+
+    jtype jt = to_jtype(class_get_cp_field_type(fieldOp.enclClass, fieldOp.cpIndex));
+    
+    const char* fieldDescName = const_pool_get_field_descriptor(fieldOp.enclClass, fieldOp.cpIndex);
+    bool fieldIsMagic = is_magic_class(fieldDescName);
+    if (fieldIsMagic) {
+        jt = iplatf;
+    }
 
     if (fieldOp.isPut() && compilation_params.exe_notify_field_modification && !fieldIsMagic)  {
         gen_modification_watchpoint(fieldOp.opcode, jt, fieldOp.fld);
@@ -290,9 +292,12 @@
         gen_access_watchpoint(fieldOp.opcode, jt, fieldOp.fld);
     }
     if (fieldOp.isPut() && ! fieldIsMagic) {
+        Opnd where = get_field_addr(fieldOp, jt);
         gen_write_barrier(fieldOp.opcode, fieldOp.fld, where);
     }
 
+    Opnd where = get_field_addr(fieldOp, jt);
+    rlock(where);
     
 
     //generate get/put op

Modified: harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp?view=diff&rev=566044&r1=566043&r2=566044
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp (original)
+++ harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp Wed Aug 15 00:38:46 2007
@@ -420,28 +420,18 @@
     int mode = -1;
     
     if (opcode == OPCODE_PUTFIELD) {
-        //
         mode = 0;
-        //
-        assert(fieldSlotAddress.is_reg());
         baseObject = vstack(1, true);
         rlock(baseObject);
-        slotAddress = fieldSlotAddress;
-        rlock(slotAddress);
     }
     else if (opcode == OPCODE_PUTSTATIC) {
-        //
         mode = 1;
-        //
-        assert(fieldSlotAddress.is_reg());
         baseObject = Val(jobj, NULL_REF);
-        slotAddress = fieldSlotAddress;
-        rlock(slotAddress);
+        rlock(baseObject);
     }
     else if (opcode == OPCODE_AASTORE) {
-        //
         mode = 2;
-        //
+
         baseObject = vstack(2, true);
         rlock(baseObject);
         
@@ -453,15 +443,21 @@
         AR index = idx.is_imm() ? ar_x : idx.reg();
         unsigned scale = idx.is_imm() ? 0 : jtypes[jt].size;
         slotAddress = Val(jobj, valloc(jobj));
+        rlock(slotAddress);
         Opnd address(jobj, base, disp, index, scale);
         lea(slotAddress.as_opnd(), address);
-        rlock(slotAddress);
     }
     else {
         // must not happen
         assert(false);
         return;
     }
+    if (mode == 0 || mode == 1) {
+        assert(fieldSlotAddress.is_mem());
+        slotAddress = Val(jobj, valloc(jobj));
+        rlock(slotAddress);
+        lea(slotAddress.as_opnd(), fieldSlotAddress);
+    }
 
     value = vstack(0);
     runlock(slotAddress);
@@ -470,7 +466,7 @@
     Val modeArg((int)mode);
     Val metaAArg((int)0);
     Val metaBArg((int)0);
-    
+   
     gen_args(csig, 0, &baseObject, &slotAddress, &value,  &metaAArg, &metaBArg, &modeArg);
     // according to contract with CG guys, the WB code neither 
     // throws an exception nor may lead to GC - may use gen_call_novm.

Modified: harmony/enhanced/drlvm/trunk/vm/jitrino/src/translator/java/JavaByteCodeTranslator.cpp
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/jitrino/src/translator/java/JavaByteCodeTranslator.cpp?view=diff&rev=566044&r1=566043&r2=566044
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/jitrino/src/translator/java/JavaByteCodeTranslator.cpp (original)
+++ harmony/enhanced/drlvm/trunk/vm/jitrino/src/translator/java/JavaByteCodeTranslator.cpp Wed Aug 15 00:38:46 2007
@@ -1612,8 +1612,9 @@
     //
     //  Try some optimizations for System::arraycopy(...), Min, Max, Abs...
     //
-    if (translationFlags.genArrayCopyRepMove == true &&
-        genArrayCopyRepMove(methodDesc,numArgs,srcOpnds)) {
+    if (!compilationInterface.needWriteBarriers()    //genArrayCopyRepMove is not ready to work in WB mode
+        && translationFlags.genArrayCopyRepMove == true 
+        && genArrayCopyRepMove(methodDesc,numArgs,srcOpnds)) {
         return;
     } else if (translationFlags.genArrayCopy == true &&
         genArrayCopy(methodDesc,numArgs,srcOpnds)) {